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

rustc silently ignores invalid -C target-feature names #44815

Open
johnthagen opened this issue Sep 24, 2017 · 3 comments
Open

rustc silently ignores invalid -C target-feature names #44815

johnthagen opened this issue Sep 24, 2017 · 3 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@johnthagen
Copy link
Contributor

Compiling a simple Hello, World program:

$ rustc -V
rustc 1.20.0 (f3d6973f4 2017-08-27)

# Note the trailing "zzzz"
$ RUSTFLAGS=-Ctarget-feature=+crt-staticzzzz cargo build -v
   Compiling rust-test v0.1.0 (rust-test)
     Running `rustc --crate-name rust_test src/main.rs --crate-type bin --emit=dep-info,link -C debuginfo=2 -C metadata=aafe1708c049ad76 -C extra-filename=-aafe1708c049ad76 --out-dir rust-test/target/debug/deps -L dependency=rust-test/target/debug/deps '-Ctarget-feature=+crt-staticzzzz'`
    Finished dev [unoptimized + debuginfo] target(s) in 0.25 secs

Should RUSTFLAGS=-Ctarget-feature=+crt-staticzzzz be silently accepted?

If I pass RUSTFLAGS=-Ctarget-feature=+batman, a warning is printed, as expected.

'+batman' is not a recognized feature for this target (ignoring feature)

Why is trailing misspelling silently allowed?

@TimNN TimNN added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Sep 26, 2017
@zackmdavis
Copy link
Member

This is happening inside LLVM and plausibly should be filed with that project.

@hanna-kruppe
Copy link
Contributor

crt-static is not passed to LLVM (and shouldn't be). It seems as if rustc consumes not just crt-static but also other target features that have it as a prefix? This filtering looks suspicious:

let llvm_features = requested_features.filter(|f| {
!rustc_features.iter().any(|s| f.contains(s))
});

@petrochenkov
Copy link
Contributor

petrochenkov commented May 10, 2020

Also rustc:

@petrochenkov petrochenkov self-assigned this May 10, 2020
RalfJung added a commit to RalfJung/rust that referenced this issue 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.
RalfJung added a commit to RalfJung/rust that referenced this issue May 16, 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.
@crlf0710 crlf0710 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 11, 2020
@petrochenkov petrochenkov removed their assignment Jan 27, 2022
@workingjubilee workingjubilee added the A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. label Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants