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

cfg_target_feature doesn't work for all supported rustc features #50077

Closed
raphaelcohn opened this issue Apr 19, 2018 · 5 comments
Closed

cfg_target_feature doesn't work for all supported rustc features #50077

raphaelcohn opened this issue Apr 19, 2018 · 5 comments
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) 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

@raphaelcohn
Copy link

Viz,

target_feature does not work for rdrnd but does work for avx2 (rustc rustc 1.27.0-nightly (ac3c2288f 2018-04-18), Mac OS X El Capitan).

This is created with cargo rustc -- -C target-feature=+avx2

#[cfg(all(target_arch = "x86_64", target_feature = "avx2"))]
#[inline(always)]
pub fn generate_hyper_thread_safe_random_u64() -> u64
{
	println!("Hello World");
}

However, this is NOT created with cargo rustc -- -C target-feature=+rdrnd

#[cfg(all(target_arch = "x86_64", target_feature = "rdrnd"))]
#[inline(always)]
pub fn generate_hyper_thread_safe_random_u64() -> u64
{
	println!("Hello World");
}

ie, changing avx2 for rdrnd causes the function to not be compiled. Both avx2 and rdnrd are present inrustc --print target-features

@raphaelcohn
Copy link
Author

Ah-ha! The code in https://github.com/rust-lang/rust/blob/186db76159c57f4af442d8aa5e7c1a330ee0004b/src/librustc_trans/llvm_util.rs, line 133 ("rdrand", None), mis-labels rdrnd as rdrand.

So the list from rustc --print target-features diverges from that in that file. So it seems there's more than one source of truth.

Since target_feature is not yet stable, perhaps we could at least make the two lists match up?

Changing #[cfg(all(target_arch = "x86_64", target_feature = "rdrnd"))] to #[cfg(all(target_arch = "x86_64", target_feature = "rdrand"))] (rdrnd to rdrand) fixes things.

@XAMPPRocky XAMPPRocky added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-attributes Area: Attributes (`#[…]`, `#![…]`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 27, 2018
@newpavlov
Copy link
Contributor

It was discussed here. In short rdrnd is name used by LLVM for compatibility with GCC, while rdrand is a vendor name. It was decided to use vendor names for target_feature, but looks like it didn't get translated to rustc options, which I think it should.

@newpavlov
Copy link
Contributor

I think it should be closed in favor of #49653?

@raphaelcohn
Copy link
Author

@newpavlov It seems a sensible place to consolidate it to.

@raphaelcohn
Copy link
Author

Closing as it's no longer relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) 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

3 participants