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

Wasm Relaxed SIMD intrinsics need new names for LLVM 16 #1396

Closed
alexcrichton opened this issue Mar 20, 2023 · 5 comments · Fixed by #1494
Closed

Wasm Relaxed SIMD intrinsics need new names for LLVM 16 #1396

alexcrichton opened this issue Mar 20, 2023 · 5 comments · Fixed by #1494

Comments

@alexcrichton
Copy link
Member

As pointed out here and discovered here the relaxed simd intrinsics for wasm added in #1393 don't compile with LLVM 16 due to the intrinsics in LLVM being renamed. The patch needed to get the test suite working again is:

diff --git a/crates/core_arch/src/wasm32/relaxed_simd.rs b/crates/core_arch/src/wasm32/relaxed_simd.rs
index 8fe935d1..ae8346aa 100644
--- a/crates/core_arch/src/wasm32/relaxed_simd.rs
+++ b/crates/core_arch/src/wasm32/relaxed_simd.rs
@@ -17,22 +17,22 @@ extern "C" {
     #[link_name = "llvm.wasm.relaxed.trunc.unsigned.zero"]
     fn llvm_relaxed_trunc_unsigned_zero(a: simd::f64x2) -> simd::i32x4;

-    #[link_name = "llvm.wasm.fma.v4f32"]
+    #[link_name = "llvm.wasm.relaxed.madd.v4f32"]
     fn llvm_f32x4_fma(a: simd::f32x4, b: simd::f32x4, c: simd::f32x4) -> simd::f32x4;
-    #[link_name = "llvm.wasm.fms.v4f32"]
+    #[link_name = "llvm.wasm.relaxed.nmadd.v4f32"]
     fn llvm_f32x4_fms(a: simd::f32x4, b: simd::f32x4, c: simd::f32x4) -> simd::f32x4;
-    #[link_name = "llvm.wasm.fma.v2f64"]
+    #[link_name = "llvm.wasm.relaxed.madd.v2f64"]
     fn llvm_f64x2_fma(a: simd::f64x2, b: simd::f64x2, c: simd::f64x2) -> simd::f64x2;
-    #[link_name = "llvm.wasm.fms.v2f64"]
+    #[link_name = "llvm.wasm.relaxed.nmadd.v2f64"]
     fn llvm_f64x2_fms(a: simd::f64x2, b: simd::f64x2, c: simd::f64x2) -> simd::f64x2;

-    #[link_name = "llvm.wasm.laneselect.v16i8"]
+    #[link_name = "llvm.wasm.relaxed.laneselect.v16i8"]
     fn llvm_i8x16_laneselect(a: simd::i8x16, b: simd::i8x16, c: simd::i8x16) -> simd::i8x16;
-    #[link_name = "llvm.wasm.laneselect.v8i16"]
+    #[link_name = "llvm.wasm.relaxed.laneselect.v8i16"]
     fn llvm_i16x8_laneselect(a: simd::i16x8, b: simd::i16x8, c: simd::i16x8) -> simd::i16x8;
-    #[link_name = "llvm.wasm.laneselect.v4i32"]
+    #[link_name = "llvm.wasm.relaxed.laneselect.v4i32"]
     fn llvm_i32x4_laneselect(a: simd::i32x4, b: simd::i32x4, c: simd::i32x4) -> simd::i32x4;
-    #[link_name = "llvm.wasm.laneselect.v2i64"]
+    #[link_name = "llvm.wasm.relaxed.laneselect.v2i64"]
     fn llvm_i64x2_laneselect(a: simd::i64x2, b: simd::i64x2, c: simd::i64x2) -> simd::i64x2;

     #[link_name = "llvm.wasm.relaxed.min.v4f32"]
@@ -46,9 +46,9 @@ extern "C" {

     #[link_name = "llvm.wasm.relaxed.q15mulr.signed"]
     fn llvm_relaxed_q15mulr_signed(a: simd::i16x8, b: simd::i16x8) -> simd::i16x8;
-    #[link_name = "llvm.wasm.dot.i8x16.i7x16.signed"]
+    #[link_name = "llvm.wasm.relaxed.dot.i8x16.i7x16.signed"]
     fn llvm_i16x8_relaxed_dot_i8x16_i7x16_s(a: simd::i8x16, b: simd::i8x16) -> simd::i16x8;
-    #[link_name = "llvm.wasm.dot.i8x16.i7x16.add.signed"]
+    #[link_name = "llvm.wasm.relaxed.dot.i8x16.i7x16.add.signed"]
     fn llvm_i32x4_relaxed_dot_i8x16_i7x16_add_s(
         a: simd::i8x16,
         b: simd::i8x16,

but @Amanieu I wanted to ask, is there a means by which nowadays the intrinsics can have conditional definitions based on the LLVM version used? It looks like the current Rust nightly reverted back to LLVM 15 so this patch can't land on the main branch right now. I'd be happy to adjust with #[cfg_attr], though, if there's selectors for the LLVM version.

@thomcc
Copy link
Member

thomcc commented Mar 20, 2023

There's no cfg currently passed for LLVM version, although I think we've been through something similar before which we solved using a new llvm14-builtins-abi target feature (but perhaps I misunderstood what that was for).

@nikic
Copy link
Contributor

nikic commented Mar 20, 2023

This is an LLVM bug: Missing autoupgrade for the intrinsic renames.

For reference, the buggy patch was https://reviews.llvm.org/D138249.

@Amanieu
Copy link
Member

Amanieu commented Mar 20, 2023

llvm14-builtins-abi was for compiler-builtins, not stdarch. In general stdarch is extremely sensitive to the LLVM version since it directly binds to LLVM intrinsics. It is difficult to support anything other than the "current" LLVM version used by rustc.

@alexcrichton
Copy link
Member Author

Ok sounds like I'll wait for the LLVM 16 update to re-land and then I can send a PR with the above changes

@nikic
Copy link
Contributor

nikic commented Mar 20, 2023

Upstream fix: https://reviews.llvm.org/D146424

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