Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmdline: Make target features individually overridable #72094

Merged
merged 1 commit into from
May 16, 2020

Conversation

petrochenkov
Copy link
Contributor

Fixes #56527

Previously -C target-feature=+avx2 -C target-feature=+fma was equivalent to -C target-feature=+fma because the later -C target-feature option fully overridden previous -C target-feature.
With this PR -C target-feature=+avx2 -C target-feature=+fma is equivalent to -C target-feature=+avx2,+fma and the options are combined.

I'm not sure where the comma-separated features in a single option came from (clang uses a scheme with single feature per-option), but logically these features are entirely independent options.
So they should be overridable individually as well to be more useful in hierarchical build system, and more consistent with other rustc options and clang behavior as well.

Target feature options have a few other issues (#44815), but fixing those is going to be a bit more invasive.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 10, 2020
@petrochenkov
Copy link
Contributor Author

Looks like everything is assigned to matthewjasper these days.
Let's re-balance a bit.
r? @nikic

@rust-highfive rust-highfive assigned nikic and unassigned matthewjasper May 10, 2020

#[no_mangle]
pub fn foo() {
// CHECK: attributes #0 = { {{.*}}"target-features"="+sse2,-avx,+avx2,+avx,-avx2"{{.*}} }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #44815 (comment) regarding canonicalization of the feature list.
I'll try to fix this some time later together with other issues.

@petrochenkov petrochenkov added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. relnotes Marks issues that should be documented in the release notes of the next release. labels May 10, 2020
@nikic
Copy link
Contributor

nikic commented May 11, 2020

Agree that this is how it should work...

@bors r+

@bors
Copy link
Contributor

bors commented May 11, 2020

📌 Commit 00dcb66 has been approved by nikic

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2020
If some feature is specified more than once with both `+` and `-`,
then values passed later override values passed earlier. \
For example, `-C target-feature=+x,-y,+z -Ctarget-feature=-x,+y`
is equivalent to `-C target-feature=-x,+y,+z`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, what's up with the \ after periods? I see this is used elsewhere in this file, but have never encountered this Markdown syntax before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a line break.
I initially used trailing spaces for this, but it regularly confuses various people's editors, see #71716 (comment).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that makes sense! The choice of syntax for this is a bit confusing, as it has the exact opposite meaning in some programming languages like C...

RalfJung added a commit to RalfJung/rust that referenced this pull request May 15, 2020
cmdline: Make target features individually overridable

Fixes rust-lang#56527

Previously `-C target-feature=+avx2 -C target-feature=+fma` was equivalent to `-C target-feature=+fma` because the later `-C target-feature` option fully overridden previous `-C target-feature`.
With this PR `-C target-feature=+avx2 -C target-feature=+fma` is equivalent to `-C target-feature=+avx2,+fma` and the options are combined.

I'm not sure where the comma-separated features in a single option came from (clang uses a scheme with single feature per-option), but logically these features are entirely independent options.
So they should be overridable individually as well to be more useful in hierarchical build system, and more consistent with other rustc options and clang behavior as well.

Target feature options have a few other issues (rust-lang#44815), but fixing those is going to be a bit more invasive.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 16, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#72045 (Incomplete features can also be unsound)
 - rust-lang#72047 (Literal error reporting cleanup)
 - rust-lang#72060 (move `ty::List` into a new submodule)
 - rust-lang#72094 (cmdline: Make target features individually overridable)
 - rust-lang#72254 (Remove redundant backtick in error message.)

Failed merges:

r? @ghost
@bors bors merged commit 4fe6d52 into rust-lang:master May 16, 2020
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 6, 2020
While here clean up all pkglint warnings.  Changes since 1.44.1:

Version 1.45.2 (2020-08-03)
==========================

* [Fix bindings in tuple struct patterns][74954]
* [Fix track_caller integration with trait objects][74784]

[74954]: rust-lang/rust#74954
[74784]: rust-lang/rust#74784

Version 1.45.1 (2020-07-30)
==========================

* [Fix const propagation with references.][73613]
* [rustfmt accepts rustfmt_skip in cfg_attr again.][73078]
* [Avoid spurious implicit region bound.][74509]
* [Install clippy on x.py install][74457]

[73613]: rust-lang/rust#73613
[73078]: rust-lang/rust#73078
[74509]: rust-lang/rust#74509
[74457]: rust-lang/rust#74457

Version 1.45.0 (2020-07-16)
==========================

Language
--------
- [Out of range float to int conversions using `as` has been defined as a saturating
  conversion.][71269] This was previously undefined behaviour, but you can use the
   `{f64, f32}::to_int_unchecked` methods to continue using the current behaviour, which
   may be desirable in rare performance sensitive situations.
- [`mem::Discriminant<T>` now uses `T`'s discriminant type instead of always
  using `u64`.][70705]
- [Function like procedural macros can now be used in expression, pattern, and  statement
  positions.][68717] This means you can now use a function-like procedural macro
  anywhere you can use a declarative (`macro_rules!`) macro.

Compiler
--------
- [You can now override individual target features through the `target-feature`
  flag.][72094] E.g. `-C target-feature=+avx2 -C target-feature=+fma` is now
  equivalent to `-C target-feature=+avx2,+fma`.
- [Added the `force-unwind-tables` flag.][69984] This option allows
  rustc to always generate unwind tables regardless of panic strategy.
- [Added the `embed-bitcode` flag.][71716] This codegen flag allows rustc
  to include LLVM bitcode into generated `rlib`s (this is on by default).
- [Added the `tiny` value to the `code-model` codegen flag.][72397]
- [Added tier 3 support\* for the `mipsel-sony-psp` target.][72062]
- [Added tier 3 support for the `thumbv7a-uwp-windows-msvc` target.][72133]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.


Libraries
---------
- [`net::{SocketAddr, SocketAddrV4, SocketAddrV6}` now implements `PartialOrd`
  and `Ord`.][72239]
- [`proc_macro::TokenStream` now implements `Default`.][72234]
- [You can now use `char` with
  `ops::{Range, RangeFrom, RangeFull, RangeInclusive, RangeTo}` to iterate over
  a range of codepoints.][72413] E.g.
  you can now write the following;
  ```rust
  for ch in 'a'..='z' {
      print!("{}", ch);
  }
  println!();
  // Prints "abcdefghijklmnopqrstuvwxyz"
  ```
- [`OsString` now implements `FromStr`.][71662]
- [The `saturating_neg` method as been added to all signed integer primitive
  types, and the `saturating_abs` method has been added for all integer
  primitive types.][71886]
- [`Arc<T>`, `Rc<T>` now implement  `From<Cow<'_, T>>`, and `Box` now
  implements `From<Cow>` when `T` is `[T: Copy]`, `str`, `CStr`, `OsStr`,
  or `Path`.][71447]
- [`Box<[T]>` now implements `From<[T; N]>`.][71095]
- [`BitOr` and `BitOrAssign` are implemented for all `NonZero`
  integer types.][69813]
- [The `fetch_min`, and `fetch_max` methods have been added to all atomic
  integer types.][72324]
- [The `fetch_update` method has been added to all atomic integer types.][71843]

Stabilized APIs
---------------
- [`Arc::as_ptr`]
- [`BTreeMap::remove_entry`]
- [`Rc::as_ptr`]
- [`rc::Weak::as_ptr`]
- [`rc::Weak::from_raw`]
- [`rc::Weak::into_raw`]
- [`str::strip_prefix`]
- [`str::strip_suffix`]
- [`sync::Weak::as_ptr`]
- [`sync::Weak::from_raw`]
- [`sync::Weak::into_raw`]
- [`char::UNICODE_VERSION`]
- [`Span::resolved_at`]
- [`Span::located_at`]
- [`Span::mixed_site`]
- [`unix::process::CommandExt::arg0`]

Cargo
-----

Misc
----
- [Rustdoc now supports strikethrough text in Markdown.][71928] E.g.
  `~~outdated information~~` becomes "~~outdated information~~".
- [Added an emoji to Rustdoc's deprecated API message.][72014]

Compatibility Notes
-------------------
- [Trying to self initialize a static value (that is creating a value using
  itself) is unsound and now causes a compile error.][71140]
- [`{f32, f64}::powi` now returns a slightly different value on Windows.][73420]
  This is due to changes in LLVM's intrinsics which `{f32, f64}::powi` uses.
- [Rustdoc's CLI's extra error exit codes have been removed.][71900] These were
  previously undocumented and not intended for public use. Rustdoc still provides
  a non-zero exit code on errors.

Internals Only
--------------
- [Make clippy a git subtree instead of a git submodule][70655]
- [Unify the undo log of all snapshot types][69464]

[73420]: rust-lang/rust#73420
[72324]: rust-lang/rust#72324
[71843]: rust-lang/rust#71843
[71886]: rust-lang/rust#71886
[72234]: rust-lang/rust#72234
[72239]: rust-lang/rust#72239
[72397]: rust-lang/rust#72397
[72413]: rust-lang/rust#72413
[72014]: rust-lang/rust#72014
[72062]: rust-lang/rust#72062
[72094]: rust-lang/rust#72094
[72133]: rust-lang/rust#72133
[71900]: rust-lang/rust#71900
[71928]: rust-lang/rust#71928
[71662]: rust-lang/rust#71662
[71716]: rust-lang/rust#71716
[71447]: rust-lang/rust#71447
[71269]: rust-lang/rust#71269
[71095]: rust-lang/rust#71095
[71140]: rust-lang/rust#71140
[70655]: rust-lang/rust#70655
[70705]: rust-lang/rust#70705
[69984]: rust-lang/rust#69984
[69813]: rust-lang/rust#69813
[69464]: rust-lang/rust#69464
[68717]: rust-lang/rust#68717
[`Arc::as_ptr`]: https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.as_ptr
[`BTreeMap::remove_entry`]: https://doc.rust-lang.org/stable/std/collections/struct.BTreeMap.html#method.remove_entry
[`Rc::as_ptr`]: https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#method.as_ptr
[`rc::Weak::as_ptr`]: https://doc.rust-lang.org/stable/std/rc/struct.Weak.html#method.as_ptr
[`rc::Weak::from_raw`]: https://doc.rust-lang.org/stable/std/rc/struct.Weak.html#method.from_raw
[`rc::Weak::into_raw`]: https://doc.rust-lang.org/stable/std/rc/struct.Weak.html#method.into_raw
[`sync::Weak::as_ptr`]: https://doc.rust-lang.org/stable/std/sync/struct.Weak.html#method.as_ptr
[`sync::Weak::from_raw`]: https://doc.rust-lang.org/stable/std/sync/struct.Weak.html#method.from_raw
[`sync::Weak::into_raw`]: https://doc.rust-lang.org/stable/std/sync/struct.Weak.html#method.into_raw
[`str::strip_prefix`]: https://doc.rust-lang.org/stable/std/primitive.str.html#method.strip_prefix
[`str::strip_suffix`]: https://doc.rust-lang.org/stable/std/primitive.str.html#method.strip_suffix
[`char::UNICODE_VERSION`]: https://doc.rust-lang.org/stable/std/char/constant.UNICODE_VERSION.html
[`Span::resolved_at`]: https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.resolved_at
[`Span::located_at`]: https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.located_at
[`Span::mixed_site`]: https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.mixed_site
[`unix::process::CommandExt::arg0`]: https://doc.rust-lang.org/std/os/unix/process/trait.CommandExt.html#tymethod.arg0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-C target-feature=+foo overrides previous values
5 participants