search

rust-lang / stdarch

Rust's standard library vendor-specific APIs and run-time feature detection
https://doc.rust-lang.org/stable/core/arch/
Apache License 2.0
599 stars 264 forks source link

Add missing `#[target_feature(enable = "simd128")]` #1609

Closed daxpedda closed 1 month ago

daxpedda commented 1 month ago

I believe that adding these was missed in #874 and adding this now should not be a breaking change.

Cc @alexcrichton.

rustbot commented 1 month ago

r? @Amanieu

rustbot has assigned @Amanieu. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Amanieu commented 1 month ago

I believe these were intentionally omitted to allow using SIMD type constructors in a const context.

Amanieu commented 1 month ago

And in the end, just using the SIMD types doesn't require the feature to be available, the compiler will just fall back to treating it as an array of scalars.

alexcrichton commented 1 month ago

Ah yes at the time this wasn't possible due to the usage of const, but if the compiler now allows #[target_feature] on const functions I think this is ok to land. I also agree with @Amanieu though that these should be optional and not required.

daxpedda commented 1 month ago

I also agree with @Amanieu though that these should be optional and not required.

Does #[target_feature(enable = "simd128")] cause LLVM to generate SIMD128 instructions here even without using -Ctarget-feature=simd128?

alexcrichton commented 1 month ago

Right yeah, the -C flag applies to all functions within a crate and #[target_feature] only applies to a single function, and LLVM only needs it enabled for the function-at-hand.

daxpedda commented 1 month ago

I see, thanks! Will go ahead and close this then.