Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
RalfJung opened this issue Oct 26, 2024 · 6 comments · Fixed by rust-lang/rust#132174

Comments

@RalfJung
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member Author

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

@RalfJung
Copy link
Member Author

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

@Lokathor
Copy link
Contributor

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

@calebzulawski
Copy link
Member

calebzulawski commented Oct 26, 2024

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

@RalfJung
Copy link
Member Author

Okay, rust-lang/rust#132174 should do it then.

Zalathar added a commit to Zalathar/rust that referenced this issue Oct 26, 2024
x86 target features: make pclmulqdq imply sse2

Based on comments in rust-lang/stdarch#1661

Fixes rust-lang/stdarch#1661
jieyouxu added a commit to jieyouxu/rust that referenced this issue Oct 26, 2024
x86 target features: make pclmulqdq imply sse2

Based on comments in rust-lang/stdarch#1661

Fixes rust-lang/stdarch#1661
jieyouxu added a commit to jieyouxu/rust that referenced this issue Oct 26, 2024
x86 target features: make pclmulqdq imply sse2

Based on comments in rust-lang/stdarch#1661

Fixes rust-lang/stdarch#1661
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 26, 2024
Rollup merge of rust-lang#132174 - RalfJung:pclmulqdq, r=calebzulawski

x86 target features: make pclmulqdq imply sse2

Based on comments in rust-lang/stdarch#1661

Fixes rust-lang/stdarch#1661
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants