Cranelift: ensure ISA level needed for SIMD is present when SIMD is e…

…nabled. (bytecodealliance#3816) Addresses bytecodealliance#3809: when we are asked to create a Cranelift backend with shared flags that indicate support for SIMD, we should check that the ISA level needed for our SIMD lowerings is present.
avanhatt · Mar 17, 2022 · 8c10bc3 · 8c10bc3
1 parent 4aea0c9
commit 8c10bc3
Show file tree

Hide file tree

Showing 55 changed files with 172 additions and 84 deletions.
diff --git a/cranelift/codegen/meta/src/isa/x86.rs b/cranelift/codegen/meta/src/isa/x86.rs
@@ -17,19 +17,22 @@ fn define_settings(shared: &SettingGroup) -> SettingGroup {
         "has_sse3",
         "Has support for SSE3.",
         "SSE3: CPUID.01H:ECX.SSE3[bit 0]",
-        false,
+        // Needed for default `enable_simd` setting.
+        true,
     );
     let has_ssse3 = settings.add_bool(
         "has_ssse3",
         "Has support for SSSE3.",
         "SSSE3: CPUID.01H:ECX.SSSE3[bit 9]",
-        false,
+        // Needed for default `enable_simd` setting.
+        true,
     );
     let has_sse41 = settings.add_bool(
         "has_sse41",
         "Has support for SSE4.1.",
         "SSE4.1: CPUID.01H:ECX.SSE4_1[bit 19]",
-        false,
+        // Needed for default `enable_simd` setting.
+        true,
     );
     let has_sse42 = settings.add_bool(
         "has_sse42",

diff --git a/cranelift/codegen/src/isa/aarch64/inst/unwind/systemv.rs b/cranelift/codegen/src/isa/aarch64/inst/unwind/systemv.rs
@@ -85,7 +85,8 @@ mod tests {
     fn test_simple_func() {
         let isa = lookup(triple!("aarch64"))
             .expect("expect aarch64 ISA")
-            .finish(Flags::new(builder()));
+            .finish(Flags::new(builder()))
+            .expect("Creating compiler backend");
 
         let mut context = Context::for_function(create_function(
             CallConv::SystemV,
@@ -127,7 +128,8 @@ mod tests {
     fn test_multi_return_func() {
         let isa = lookup(triple!("aarch64"))
             .expect("expect aarch64 ISA")
-            .finish(Flags::new(builder()));
+            .finish(Flags::new(builder()))
+            .expect("Creating compiler backend");
 
         let mut context = Context::for_function(create_multi_return_function(CallConv::SystemV));
 

diff --git a/cranelift/codegen/src/isa/aarch64/mod.rs b/cranelift/codegen/src/isa/aarch64/mod.rs
@@ -172,7 +172,7 @@ pub fn isa_builder(triple: Triple) -> IsaBuilder {
         constructor: |triple, shared_flags, builder| {
             let isa_flags = aarch64_settings::Flags::new(&shared_flags, builder);
             let backend = AArch64Backend::new_with_flags(triple, shared_flags, isa_flags);
-            Box::new(backend)
+            Ok(Box::new(backend))
         },
     }
 }

diff --git a/cranelift/codegen/src/isa/mod.rs b/cranelift/codegen/src/isa/mod.rs
@@ -139,7 +139,8 @@ impl fmt::Display for LookupError {
 pub struct Builder {
     triple: Triple,
     setup: settings::Builder,
-    constructor: fn(Triple, settings::Flags, settings::Builder) -> Box<dyn TargetIsa>,
+    constructor:
+        fn(Triple, settings::Flags, settings::Builder) -> CodegenResult<Box<dyn TargetIsa>>,
 }
 
 impl Builder {
@@ -153,9 +154,13 @@ impl Builder {
         self.setup.iter()
     }
 
-    /// Combine the ISA-specific settings with the provided ISA-independent settings and allocate a
-    /// fully configured `TargetIsa` trait object.
-    pub fn finish(self, shared_flags: settings::Flags) -> Box<dyn TargetIsa> {
+    /// Combine the ISA-specific settings with the provided
+    /// ISA-independent settings and allocate a fully configured
+    /// `TargetIsa` trait object. May return an error if some of the
+    /// flags are inconsistent or incompatible: for example, some
+    /// platform-independent features, like general SIMD support, may
+    /// need certain ISA extensions to be enabled.
+    pub fn finish(self, shared_flags: settings::Flags) -> CodegenResult<Box<dyn TargetIsa>> {
         (self.constructor)(self.triple, shared_flags, self.setup)
     }
 }

diff --git a/cranelift/codegen/src/isa/s390x/inst/unwind/systemv.rs b/cranelift/codegen/src/isa/s390x/inst/unwind/systemv.rs
@@ -100,7 +100,8 @@ mod tests {
     fn test_simple_func() {
         let isa = lookup(triple!("s390x"))
             .expect("expect s390x ISA")
-            .finish(Flags::new(builder()));
+            .finish(Flags::new(builder()))
+            .expect("Creating compiler backend");
 
         let mut context = Context::for_function(create_function(
             CallConv::SystemV,
@@ -142,7 +143,8 @@ mod tests {
     fn test_multi_return_func() {
         let isa = lookup(triple!("s390x"))
             .expect("expect s390x ISA")
-            .finish(Flags::new(builder()));
+            .finish(Flags::new(builder()))
+            .expect("Creating compiler backend");
 
         let mut context = Context::for_function(create_multi_return_function(
             CallConv::SystemV,

diff --git a/cranelift/codegen/src/isa/s390x/mod.rs b/cranelift/codegen/src/isa/s390x/mod.rs
@@ -179,7 +179,7 @@ pub fn isa_builder(triple: Triple) -> IsaBuilder {
         constructor: |triple, shared_flags, builder| {
             let isa_flags = s390x_settings::Flags::new(&shared_flags, builder);
             let backend = S390xBackend::new_with_flags(triple, shared_flags, isa_flags);
-            Box::new(backend)
+            Ok(Box::new(backend))
         },
     }
 }

diff --git a/cranelift/codegen/src/isa/x64/inst/unwind/systemv.rs b/cranelift/codegen/src/isa/x64/inst/unwind/systemv.rs
@@ -112,7 +112,8 @@ mod tests {
     fn test_simple_func() {
         let isa = lookup(triple!("x86_64"))
             .expect("expect x86 ISA")
-            .finish(Flags::new(builder()));
+            .finish(Flags::new(builder()))
+            .expect("expect backend creation to succeed");
 
         let mut context = Context::for_function(create_function(
             CallConv::SystemV,
@@ -154,7 +155,8 @@ mod tests {
     fn test_multi_return_func() {
         let isa = lookup(triple!("x86_64"))
             .expect("expect x86 ISA")
-            .finish(Flags::new(builder()));
+            .finish(Flags::new(builder()))
+            .expect("expect backend creation to succeed");
 
         let mut context = Context::for_function(create_multi_return_function(CallConv::SystemV));
 

diff --git a/cranelift/codegen/src/isa/x64/mod.rs b/cranelift/codegen/src/isa/x64/mod.rs
@@ -11,7 +11,7 @@ use crate::isa::Builder as IsaBuilder;
 use crate::machinst::{
     compile, MachCompileResult, MachTextSectionBuilder, TextSectionBuilder, VCode,
 };
-use crate::result::CodegenResult;
+use crate::result::{CodegenError, CodegenResult};
 use crate::settings::{self as shared_settings, Flags};
 use alloc::{boxed::Box, vec::Vec};
 use core::fmt;
@@ -174,10 +174,21 @@ fn isa_constructor(
     triple: Triple,
     shared_flags: Flags,
     builder: shared_settings::Builder,
-) -> Box<dyn TargetIsa> {
+) -> CodegenResult<Box<dyn TargetIsa>> {
     let isa_flags = x64_settings::Flags::new(&shared_flags, builder);
+
+    // Check for compatibility between flags and ISA level
+    // requested. In particular, SIMD support requires SSE4.1.
+    if shared_flags.enable_simd() {
+        if !isa_flags.has_sse3() || !isa_flags.has_ssse3() || !isa_flags.has_sse41() {
+            return Err(CodegenError::Unsupported(
+                "SIMD support requires SSE3, SSSE3, and SSE4.1 on x86_64.".into(),
+            ));
+        }
+    }
+
     let backend = X64Backend::new_with_flags(triple, shared_flags, isa_flags);
-    Box::new(backend)
+    Ok(Box::new(backend))
 }
 
 #[cfg(test)]
@@ -332,4 +343,20 @@ mod test {
 
         assert_eq!(code, &golden[..]);
     }
+
+    // Check that feature tests for SIMD work correctly.
+    #[test]
+    fn simd_required_features() {
+        let mut shared_flags_builder = settings::builder();
+        shared_flags_builder.set("enable_simd", "true").unwrap();
+        let shared_flags = settings::Flags::new(shared_flags_builder);
+        let mut isa_builder = crate::isa::lookup_by_name("x86_64").unwrap();
+        isa_builder.set("has_sse3", "false").unwrap();
+        isa_builder.set("has_ssse3", "false").unwrap();
+        isa_builder.set("has_sse41", "false").unwrap();
+        assert!(matches!(
+            isa_builder.finish(shared_flags),
+            Err(CodegenError::Unsupported(_)),
+        ));
+    }
 }
diff --git a/cranelift/filetests/filetests/isa/x64/simd-lane-access-compile.clif b/cranelift/filetests/filetests/isa/x64/simd-lane-access-compile.clif
@@ -1,6 +1,6 @@
 test compile precise-output
 set enable_simd
-target x86_64 has_ssse3 has_sse41
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 ;; shuffle
 

diff --git a/cranelift/filetests/filetests/runtests/simd-bitselect.clif b/cranelift/filetests/filetests/runtests/simd-bitselect.clif
@@ -1,7 +1,7 @@
 test run
 set enable_simd
 target aarch64
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %bitselect_i32x4(i32x4, i32x4, i32x4) -> i32x4 {
 block0(v0: i32x4, v1: i32x4, v2: i32x4):

diff --git a/cranelift/filetests/filetests/runtests/simd-comparison.clif b/cranelift/filetests/filetests/runtests/simd-comparison.clif
@@ -2,7 +2,7 @@ test run
 target aarch64
 ; target s390x TODO: Not yet implemented on s390x
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %icmp_eq_i8x16() -> b8 {
 block0:

diff --git a/cranelift/filetests/filetests/runtests/simd-conversion.clif b/cranelift/filetests/filetests/runtests/simd-conversion.clif
@@ -2,7 +2,7 @@ test run
 target aarch64
 ; target s390x TODO: Not yet implemented on s390x
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %fcvt_from_sint(i32x4) -> f32x4 {
 block0(v0: i32x4):

diff --git a/cranelift/filetests/filetests/runtests/simd-extractlane.clif b/cranelift/filetests/filetests/runtests/simd-extractlane.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %extractlane_4(i8x16) -> i8 {
 block0(v0: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-iabs.clif b/cranelift/filetests/filetests/runtests/simd-iabs.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %iabs_i8x16(i8x16) -> i8x16 {
 block0(v0: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-insertlane.clif b/cranelift/filetests/filetests/runtests/simd-insertlane.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %insertlane_15(i8x16, i8) -> i8x16 {
 block0(v0: i8x16, v1: i8):

diff --git a/cranelift/filetests/filetests/runtests/simd-lane-access.clif b/cranelift/filetests/filetests/runtests/simd-lane-access.clif
@@ -2,7 +2,7 @@ test run
 target aarch64
 ; target s390x TODO: Not yet implemented on s390x
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 ;; shuffle
 

diff --git a/cranelift/filetests/filetests/runtests/simd-logical.clif b/cranelift/filetests/filetests/runtests/simd-logical.clif
@@ -2,7 +2,7 @@ test run
 target aarch64
 ; target s390x TODO: Not yet implemented on s390x
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %bnot() -> b32 {
 block0:

diff --git a/cranelift/filetests/filetests/runtests/simd-saddsat.clif b/cranelift/filetests/filetests/runtests/simd-saddsat.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %saddsat_i8x16(i8x16, i8x16) -> i8x16 {
 block0(v0: i8x16, v1: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-shuffle.clif b/cranelift/filetests/filetests/runtests/simd-shuffle.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %shuffle_i8x16(i8x16, i8x16) -> i8x16 {
 block0(v0: i8x16, v1: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-snarrow.clif b/cranelift/filetests/filetests/runtests/simd-snarrow.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %snarrow_i16x8(i16x8, i16x8) -> i8x16 {
 block0(v0: i16x8, v1: i16x8):

diff --git a/cranelift/filetests/filetests/runtests/simd-splat.clif b/cranelift/filetests/filetests/runtests/simd-splat.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %splat_i8x16(i8) -> i8x16 {
 block0(v0: i8):

diff --git a/cranelift/filetests/filetests/runtests/simd-sqmulroundsat.clif b/cranelift/filetests/filetests/runtests/simd-sqmulroundsat.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %sqmulrs_i16x8(i16x8, i16x8) -> i16x8 {
 block0(v0: i16x8, v1: i16x8):

diff --git a/cranelift/filetests/filetests/runtests/simd-ssubsat.clif b/cranelift/filetests/filetests/runtests/simd-ssubsat.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %ssubsat_i8x16(i8x16, i8x16) -> i8x16 {
 block0(v0: i8x16, v1: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-swidenhigh.clif b/cranelift/filetests/filetests/runtests/simd-swidenhigh.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %swidenhigh_i8x16(i8x16) -> i16x8 {
 block0(v0: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-swidenlow.clif b/cranelift/filetests/filetests/runtests/simd-swidenlow.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %swidenlow_i8x16(i8x16) -> i16x8 {
 block0(v0: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-swizzle.clif b/cranelift/filetests/filetests/runtests/simd-swizzle.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %swizzle_i8x16(i8x16, i8x16) -> i8x16 {
 block0(v0: i8x16, v1: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-uaddsat.clif b/cranelift/filetests/filetests/runtests/simd-uaddsat.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %uaddsat_i8x16(i8x16, i8x16) -> i8x16 {
 block0(v0: i8x16, v1: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-unarrow.clif b/cranelift/filetests/filetests/runtests/simd-unarrow.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %unarrow_i16x8(i16x8, i16x8) -> i8x16 {
 block0(v0: i16x8, v1: i16x8):

diff --git a/cranelift/filetests/filetests/runtests/simd-usubsat.clif b/cranelift/filetests/filetests/runtests/simd-usubsat.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %usubsat_i8x16(i8x16, i8x16) -> i8x16 {
 block0(v0: i8x16, v1: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-uwidenhigh.clif b/cranelift/filetests/filetests/runtests/simd-uwidenhigh.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %uwidenhigh_i8x16(i8x16) -> i16x8 {
 block0(v0: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-uwidenlow.clif b/cranelift/filetests/filetests/runtests/simd-uwidenlow.clif
@@ -2,7 +2,7 @@ test interpret
 test run
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 function %uwidenlow_i8x16(i8x16) -> i16x8 {
 block0(v0: i8x16):

diff --git a/cranelift/filetests/filetests/runtests/simd-vconst.clif b/cranelift/filetests/filetests/runtests/simd-vconst.clif
@@ -2,7 +2,7 @@ test run
 ; target s390x TODO: Not yet implemented on s390x
 target aarch64
 set enable_simd
-target x86_64
+target x86_64 has_sse3 has_ssse3 has_sse41
 
 
 function %vconst_zeroes() -> b1 {