RFC: Changes to target_feature attribute

[veluca93]

tldr: make it easier/safer to use target_feature in traits.

Also see prior discussion in rust-lang/rust#139368.

Rendered

[veluca93]

@hanna-kruppe @traviscross @RalfJung

Correct RFC link here :-)

[hanna-kruppe]

If the language had something like #3525 to make the "function takes argument that ensures the target feature is available" reasoning transparent to the compiler, would we still want this form of the attribute? Would the attribute still exist but no longer be unsafe? Or would we keep it unsafe but deprecate it in favor of the fully safe way with parameter types?

[veluca93]

This likely depends on the shape #3525 would take (if any -- note that discussion on that RFC has been effectively stalled for quite some time).

The fundamental issue at hand here is that it's hard to define "function takes argument that ensures" at a language level in a way that is both ergonomic and doesn't run into recursive reference validity issues.

IMO, the attribute is likely to still be useful to bridge cases that are unlikely to be solved at language level, such as fn f(_: &Avx2).

There are also some other practical issues for #3525 (involving generics and the MIR inliner).

My understanding is that #3525 as-is is somewhat unlikely to actually be accepted, and the most likely way to achieve SIMD multiversioning functionality will be through some version of an effect system; however, those discussions are at an early enough stage that it would be hard for me to claim with any confidence whether an attribute like unsafe(target_feature) would still have an use then. If not, it can always be deprecated :-)

[hanna-kruppe]

I get that the details of the compiler establishing safety of extra target features via arguments' types are very tricky. But it would be extremely unfortunate to first drag the ecosystem through the churn of a FCW, and then one edition later deprecate the style that the FCW promoted in favor of something new that solves the same use case better. (Of course, it would also be unfortunate to block any improvement here on some speculative far-future language evolution.)

[RalfJung]

How does this account for what aho-corasick does? It seems to overfit specifically to TC's example, but while the example demonstrates the issue from rust-lang/rust#139368, it does not seem to capture every way in which existing code could soundly use target_feature in traits.

Someone needs to dig into aho-corasick and figure out the exact flow of proof obligations there. And, ideally, go hunt for some other examples across the ecosystem.

[veluca93]

From what I can see, the pattern in aho-corasick matches what would be possible with #[unsafe(target_feature(force=...))]. In particular, SearcherT is implemented for 4 (generic) structs named (Slim|Fat)(Avx2|Ssse3), which check for presence of the relevant CPU features upon construction and then declare in the trait method implementation...

impl SearcherT for FatAVX2<$len> {
    #[target_feature(enable = "avx2")]
    #[inline]
    unsafe fn find(
        &self,
        start: *const u8,
        end: *const u8,
    ) -> Option<Match> {
        // SAFETY: All obligations except for `target_feature` are
        // passed to the caller. Our use of `target_feature` is
        // safe because construction of this type requires that the
        // requisite target features are available.
        self.fat256.find(start, end)
    }
}

(admittedly the location of the safety comment wrt features is not quite correct, but alas)

[RalfJung]

Ah, nice.