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
612 stars 271 forks source link

abi_unsupported_vector_types triggering in pclmulqdq (and maybe more places) #1661

Open RalfJung opened 3 weeks ago

RalfJung commented 3 weeks ago
error: ABI error: this function definition uses a vector type that requires the `sse` target feature, which is not enabled
  --> crates/core_arch/src/x86/pclmulqdq.rs:28:18
   |
28 | #[cfg_attr(test, assert_instr(pclmul, IMM8 = 0))]
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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")]`)
   = note: `-D abi-unsupported-vector-types` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(abi_unsupported_vector_types)]`
   = note: this error originates in the attribute macro `assert_instr` (in Nightly builds, run with -Z macro-backtrace for more info)

error: ABI error: this function call uses a vector type that requires the `sse` target feature, which is not enabled in the caller
  --> crates/core_arch/src/x86/pclmulqdq.rs:33:5
   |
33 |     pclmulqdq(a, b, IMM8 as u8)
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ function called 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")]`)

This is the following function:

extern "C" {
    #[link_name = "llvm.x86.pclmulqdq"]
    fn pclmulqdq(a: __m128i, round_key: __m128i, imm8: u8) -> __m128i;
}

#[target_feature(enable = "pclmulqdq")]
pub unsafe fn _mm_clmulepi64_si128<const IMM8: i32>(a: __m128i, b: __m128i) -> __m128i {
    static_assert_uimm_bits!(IMM8, 8);
    pclmulqdq(a, b, IMM8 as u8) // ERROR
}

Does the pclmulqdq target feature imply sse?

And shouldn't this use extern "unadjusted" like most of the LLVM intrinsics?

Cc @Amanieu @veluca93

veluca93 commented 3 weeks ago

I imagine it should - since the intrinsic operates on SSE registers, it would be pretty hard to do anything with it if SSE is not available...

RalfJung commented 3 weeks ago

So this seems to be missing info in rustc's feature implication code then? Cc @calebzulawski

RalfJung commented 3 weeks ago

Specifically that would be adding sse in this line. But I don't know how to check whether that is correct.

Lokathor commented 3 weeks ago

you could double check with llvm's feature implication code maybe

calebzulawski commented 3 weeks ago

LLVM appears to have pclmul depend on sse2: https://github.com/llvm/llvm-project/blob/3fc0d94ce57de2d0841e77c8fda7feef2923c4e0/llvm/lib/Target/X86/X86.td#L187

RalfJung commented 3 weeks ago

Okay, https://github.com/rust-lang/rust/pull/132174 should do it then.