Tracking issue for RFC 2045: improving #[target_feature]

[alexcrichton]

This is a tracking issue for the #[target_feature] segment of RFC 2045 (rust-lang/rfcs#2045).
#[cfg(target_feature)] was tracked in #29717 and has since been stabilized.

Steps

• Implement the proposed #[target_feature] semantics
• Document these semantics: Document target_feature and cfg_target_feature. reference#545
• Stabilize
  • x86_64 & i686
    • The basic set (sse–sse4.2, avx, avx2, ...) (Stabilize x86/x86_64 SIMD #49664)
    • adx: Stabilize ADX target feature #93745
    • avx512
    • mmx (never going to be stabilized)
    • sse4a
    • tbm
    • lahfsahf
    • prfchw
  • arm
  • aarch64
  • hexagon
  • powerpc
  • mips

@gnzlbg have anything else you want filled out here?

(below added from comments on PR)

• consensus on the API for run-time feature detection
• should cfg!(feature) work across #[inline(always)] functions, generics, etc?

And some related tasks:

• API breaking change: allow #[target_feature] on unsafe functions only
• API breaking change: #[target_feature = "+feature"] => #[target_feature(enable = "feature")]
• fix bug: cfg_target_feature and target_feature don't interact properly #42515
• resolve bug: repr(simd) is unsound #44367

[gnzlbg]

@alexchrichton

Yes, we should also clear some of the unresolved questions:

• consensus on the API for run-time feature detection
• should cfg!(feature) work across #[inline(always)] functions, generics, etc?

And some related tasks:

• API breaking change: allow #[target_feature] on unsafe functions only
• API breaking change: #[target_feature = "+feature"] => #[target_feature(enable = "feature")]
• fix bug: cfg_target_feature and target_feature don't interact properly #42515
• resolve bug: repr(simd) is unsound #44367

It would be nice if those API breaking changes could be prioritized.

[retep998]

Rust already will sometimes not inline a #[inline(always)] function (notably recursive situations), so unsafe code already can't assume that #[inline(always)] code will always be inlined.

[gnzlbg]

@retep998 cfg!(feature) does not require any unsafe code.

[retep998]

@gnzlbg I meant in regards to where the RFC states that #[target_feature] and #[inline(always)] mixed together should result in an error. We can simply not inline functions in that case, even if they are marked as #[inline(always)], because Rust will already sometimes not inline functions marked #[inline(always)].

[gnzlbg]

@retep998 We could definitely relax this by just not inlining an #[inline(always)] function. I don't know if we want to do so: the user did not write inline, but inline(always), we should at least warn here.

What we cannot currently do is inlining a function across mismatching features (independently of its inlining annotations). The alternative sections of the RFC explores this a bit though.

[newpavlov]

Any news on implementation status of --enable-features="feature0,feature1,..." proposed in the RFC?

[gnzlbg]

@newpavlov this is being discussed in issue 49658 . (note from pnkfelix: did this mean to say #49653 ?)

[alexcrichton]

#48556 has entered FCP which I believe will implicitly stabilize this attribute

[gnzlbg]

So after the inline-always fix it looks like the only bug open still being tracked here is #42515 .

[gnzlbg]

That bug looks hard to solve and probably won't make it before stabilization.

The following issues look "easy" to fix low-hanging fruit. It might be worth to fix these before stabilization:

• Should -Ctarget-feature go straight to LLVM? #49653
• rustc silently ignores invalid -C target-feature names #44815
• LLVM assert with invalid -C target-feature #37704

[nagisa]

API breaking change: allow #[target_feature] on unsafe functions only
API breaking change: #[target_feature = "+feature"] => #[target_feature(enable = "feature")]

Both of these are true today, except for the first on WASM platforms, where #[target_feature] was made applicable to safe functions knowingly.

[bkolligs]

Any update on AVX 512 support?

[qwandor]

Hi, is there anything blocking stabilising arm_target_feature?

[coastalwhite]

Hey! I am currently implementing RISC-V Zk intrinsics, and I am running into a pattern that is currently not well covered by this PR.

Either Zkne and Zknd extension needs to be enabled to get access to an intrinsic. I imagine for the compiler it is enough to write:

#[target_feature(enable = "zkne")]
pub unsafe fn aes64ks1i(...) -> ... {
     ...
}

But this produces wrong cargo doc output and can be misleading. Namely, it states that zkne needs to be enabled.

#[target_feature(enable = "zkne", enable = "zknd")]
// OR
#[target_feature(enable = "zkne")]
#[target_feature(enable = "zknd")]
pub unsafe fn aes64ks1i(...) -> ... {
     ...
}

Produces zkne AND zknd.

#[target_feature(enable = "zkne,zknd")]
pub unsafe fn aes64ks1i(...) -> ... {
     ...
}

Produces zkne,zknd which does not exist.

I feel like this is only a documentation issue, but still an issue.

[majaha]

I'm not sure if this is the right place for this, but the wasm feature #[target_feature(enable = "multivalue")] is broken at the moment, as it leaks into other untagged functions: Godbolt
This is essentially for the same reasons I've outline here: #83788 (comment)
There may be other target features with similar non-local behaviour, I haven't tested them all.

[jhorstmann]

I tried reading through the open issues regarding target features, but could not find a more fitting one for this question. I have the following code, which currently returns different values on stable vs nightly when compiled with -Ctarget-cpu=skx (Godbolt). I would have expected that target-cpu implicitly enables that feature, or a warning/error that the avx512 target features are still unstable.

pub fn preferred_vec_size() -> usize {
    if cfg!(all(target_arch = "x86_64", target_feature = "avx512f")) {
        64
    } else if cfg!(all(target_arch = "x86_64", target_feature="avx")) {
        32
    } else {
        16
    }
}

[workingjubilee]

That's... huh. I guess the first branch is being ignored as an invalid feature gate on stable.

[tarcieri]

Are there any particular stabilization blockers for feature(avx512_target_feature), or does it just need a stabilization PR?