new warnings about `#[inline(always)]` functions missing `#[target_feature(enable="sse")]`

[oconnor663]

Apologies if this code was always broken and I just don't know it yet :) Here's a CI run that recently started failing, due to new warnings plus our usual RUSTFLAGS: "-D warnings": https://github.com/BLAKE3-team/BLAKE3/actions/runs/15326026728/job/43120654065. Reproducing that locally:

$ git clone https://github.com/BLAKE3-team/BLAKE3                                                                                                  
...
$ cd BLAKE3                                                                                                                                        
$ cross rustc --release --target i686-unknown-linux-musl -- -C target-cpu=i386                                                                     
...
warning: this function definition uses SIMD vector type `std::arch::x86::__m128i` which (with the chosen ABI) requires the `sse` target feature, wh
ich is not enabled                                                                                                                                 
  --> src/rust_sse2.rs:75:1                                                                                                                        
   |                                                                                                                                               
75 | / unsafe fn g1(                                                                                                                               
76 | |     row0: &mut __m128i,                                                                                                                     
77 | |     row1: &mut __m128i,                                                                                                                     
78 | |     row2: &mut __m128i,                                                                                                                     
79 | |     row3: &mut __m128i,                                                                                                                     
80 | |     m: __m128i,                                                                                                                             
81 | | ) {                                                                                                                                         
   | |_^ function defined here
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #116558 <https://github.com/rust-lang/rust/issues/116558>
   = help: consider enabling it globally (`-C target-feature=+sse`) or locally (`#[target_feature(enable="sse")]`)
...

The code in that file is divided into two types of functions, public ones annotated with #[target_feature(enable = "sse2")], and private ones annotated with #[inline(always)]. I had thought (or read somewhere) that inline(always) functions assumed the CPU features of their caller and didn't need to be annotated. Either way it seems to be incompatible with target_feature. What's should this code be doing differently?

[RalfJung]

I had thought (or read somewhere) that inline(always) functions assumed the CPU features of their caller and didn't need to be annotated

In terms of using the target features inside the function, this is correct as long as all callers have the attribute.

However, enabling target features also affects the ABI (that's not our choice, it's a fundamental design issue in LLVM we can just try to work around), and there's no "inheritance" of target features there. And inline(always) is just a hint. If it ever gets ignored, and the caller has the target feature while the callee does not, then passing that argument by-value can go wrong in the least hilarious ways. So the warning is right, there's a potential problem here.

Ideally you'd just have the target feature on this function as well, but sadly LLVM's handling of target features during inlining still has tons of problems (precisely as a consequence of the aforementioned design issue) so we forbid them on inline(always) functions.

The easiest way tog et rid of the warning is to use the "Rust" ABI rather than "C". Since this seems to be entirely an internal helper function -- why are you using "C"?

Beyond that, I don't know what the best way is to write this that is both sound, supported by LLVM, and well-optimized. Judging from things like #139029, it seems generally advisable to pass vector arguments in-memory rather than by-register if you want things to be inlined by LLVM -- so changing the last argument to &__m128i might be a good idea. LLVM should be able to clean that up after inlining.

Cc @nikic @taiki-e @tgross35

[RalfJung]

Uh, wait, something is very strange here. You are not using the "C" ABI it seems? It shouldn't be possible to hit this warning with the "Rust" ABI...

Oh. You are using -C target-cpu=i386. That should lead to warnings by itself. You can't disable the SSE/SSE2 target features on i686 targets.

[RalfJung]

So the actual problem is this:

warning: target feature `sse2` must be enabled to ensure that the ABI of the current target can be implemented correctly
  |
  = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>

This warning is emitted so early it's not affected by -D warnings.

That also explains why you only started seeing this recently.

The "good" news is that #141309 will make the errors go away.

But the warning I just quoted will stay, and we do plan to make it a hard error eventually. Building code without SSE2 should use an i586 target, which are generally lower-tier reflecting the fact that our support for such CPUs is not great (not least because LLVM has a long-standing soundness issue for such targets caused by use of the x87 FPU -- there's apparently not enough people interested in such retro chips to keep them well-supported).