Run the full test suite on 32-bit platforms (bytecodealliance#9837)

* Run the full test suite on 32-bit platforms

This commit switches to running the full test suite in its entirety
(`./ci/run-tests.sh`) on 32-bit platforms in CI in addition to 64-bit
platforms. This notably adds i686 and armv7 as architectures that are
tested in CI.

Lots of little fixes here and there were applied to a number of tests.
Many tests just don't run on 32-bit platforms or a platform without
Cranelift support, and they've been annotated as such where necessary.
Other tests were adjusted to run on all platforms a few minor bug fixes
are here as well.

prtest:full

* Fix clippy warning

* Get wasm code working by default on 32-bit

Don't require the `pulley` feature opt-in on 32-bit platforms to get
wasm code running.

* Fix dead code warning

* Fix build on armv7

* Fix test assertion on armv7

* Review comments

* Update how tests are skipped

* Change how Pulley is defaulted

Default to pulley in `build.rs` rather than in `Cargo.toml` to make it
easier to write down the condition and comment what's happening. This
means that the `pulley-interpreter` crate and pulley support in
Cranelift is always compiled in now and cannot be removed. This should
hopefully be ok though as the `pulley-interpreter` crate is still
conditionally used (meaning it can get GC'd) and the code-size of
Cranelift is not as important as the runtime itself.

* pulley: Save/restore callee-save state on traps

* Fewer clippy warnings about casts

* Use wrapping_add in `g32_addr`, fixing arm test

Author: alexcrichton

Cargo.lock

@@ -124,6 +124,7 @@ wasmtime-test-macros = { path = "crates/test-macros" }
 pulley-interpreter = { workspace = true, features = ["disas"] }
 wasmtime-wast-util = { path = 'crates/wast-util' }
 wasm-encoder = { workspace = true }
+cranelift-native = { workspace = true }
 
 [target.'cfg(windows)'.dev-dependencies]
 windows-sys = { workspace = true, features = ["Win32_System_Memory"] }

Cargo.toml

@@ -30,15 +30,6 @@ const FAST_MATRIX = [
   },
 ];
 
-// Returns whether the given package supports a 32-bit architecture, used when
-// testing on i686 and armv7 below.
-function supports32Bit(pkg) {
-  if (pkg.indexOf("pulley") !== -1)
-    return true;
-
-  return pkg == 'wasmtime-fiber' || pkg == 'wasmtime';
-}
-
 // This is the full, unsharded, and unfiltered matrix of what we test on
 // CI. This includes a number of platforms and a number of cross-compiled
 // targets that are emulated with QEMU. This must be kept tightly in sync with
@@ -138,15 +129,13 @@ const FULL_MATRIX = [
   },
   {
     "name": "Tests on i686-unknown-linux-gnu",
-    "32-bit": true,
     "os": ubuntu,
     "target": "i686-unknown-linux-gnu",
     "gcc_package": "gcc-i686-linux-gnu",
     "gcc": "i686-linux-gnu-gcc",
   },
   {
     "name": "Tests on armv7-unknown-linux-gnueabihf",
-    "32-bit": true,
     "os": ubuntu,
     "target": "armv7-unknown-linux-gnueabihf",
     "gcc_package": "gcc-arm-linux-gnueabihf",
@@ -230,29 +219,6 @@ async function shard(configs) {
   // created above.
   const sharded = [];
   for (const config of configs) {
-    // Special case 32-bit configs. Only some crates, according to
-    // `supports32Bit`, run on this target. At this time the set of supported
-    // crates is small enough that they're not sharded. A second shard, however,
-    // is included which runs `--test wast` to run the full `*.wast` test suite
-    // in CI on 32-bit platforms, at this time effectively testing Pulley.
-    if (config["32-bit"] === true) {
-      sharded.push(Object.assign(
-        {},
-        config,
-        {
-          bucket: members
-            .map(c => supports32Bit(c) ? `--package ${c}` : `--exclude ${c}`)
-            .join(" "),
-        }
-      ));
-      sharded.push(Object.assign(
-        {},
-        config,
-        { bucket: '--test wast' },
-      ));
-      continue;
-    }
-
     for (const bucket of buckets) {
       sharded.push(Object.assign(
         {},
@@ -311,16 +277,6 @@ async function main() {
         return true;
     }
 
-    // For matrix entries that represent 32-bit only some crates support that,
-    // so whenever the crates are changed be sure to run 32-bit tests on PRs
-    // too.
-    if (config["32-bit"] === true) {
-      if (names.includes("pulley"))
-        return true;
-      if (names.includes("fiber"))
-        return true;
-    }
-
     // If the commit explicitly asks for this test config, then include it.
     if (config.filter && commits.includes(`prtest:${config.filter}`)) {
       return true;

ci/build-test-matrix.js

@@ -53,9 +53,10 @@ fn main() {
     if isas.is_empty() || host_isa {
         // Try to match native target.
         let target_name = target_triple.split('-').next().unwrap();
-        let isa = meta::isa_from_arch(&target_name).expect("error when identifying target");
-        println!("cargo:rustc-cfg=feature=\"{isa}\"");
-        isas.push(isa);
+        if let Ok(isa) = meta::isa_from_arch(&target_name) {
+            println!("cargo:rustc-cfg=feature=\"{isa}\"");
+            isas.push(isa);
+        }
     }
 
     let cur_dir = env::current_dir().expect("Can't access current working directory");

cranelift/codegen/build.rs

@@ -3051,6 +3051,11 @@ mod tests {
     fn inst_size_test() {
         // This test will help with unintentionally growing the size
         // of the Inst enum.
-        assert_eq!(32, std::mem::size_of::<Inst>());
+        let expected = if cfg!(target_pointer_width = "32") && !cfg!(target_arch = "arm") {
+            28
+        } else {
+            32
+        };
+        assert_eq!(expected, std::mem::size_of::<Inst>());
     }
 }

cranelift/codegen/src/isa/aarch64/inst/mod.rs

@@ -481,15 +481,19 @@ where
             }
             // "far" calls are pulley->host calls so they use a different opcode
             // which is lowered with a special relocation in the backend.
-            CallDest::ExtName(name, RelocDistance::Far) => smallvec![Inst::IndirectCallHost {
-                info: Box::new(info.map(|()| name.clone()))
+            CallDest::ExtName(name, RelocDistance::Far) => {
+                smallvec![Inst::IndirectCallHost {
+                    info: Box::new(info.map(|()| name.clone()))
+                }
+                .into()]
             }
-            .into()],
             // Indirect calls are all assumed to be pulley->pulley calls
-            CallDest::Reg(reg) => smallvec![Inst::IndirectCall {
-                info: Box::new(info.map(|()| XReg::new(*reg).unwrap()))
+            CallDest::Reg(reg) => {
+                smallvec![Inst::IndirectCall {
+                    info: Box::new(info.map(|()| XReg::new(*reg).unwrap()))
+                }
+                .into()]
             }
-            .into()],
         }
     }
 

cranelift/codegen/src/isa/pulley_shared/abi.rs

@@ -1749,11 +1749,11 @@ mod test {
     fn size_of_constant_structs() {
         assert_eq!(size_of::<Constant>(), 4);
         assert_eq!(size_of::<VCodeConstant>(), 4);
-        assert_eq!(size_of::<ConstantData>(), 24);
-        assert_eq!(size_of::<VCodeConstantData>(), 32);
+        assert_eq!(size_of::<ConstantData>(), 3 * size_of::<usize>());
+        assert_eq!(size_of::<VCodeConstantData>(), 4 * size_of::<usize>());
         assert_eq!(
             size_of::<PrimaryMap<VCodeConstant, VCodeConstantData>>(),
-            24
+            3 * size_of::<usize>()
         );
         // TODO The VCodeConstants structure's memory size could be further optimized.
         // With certain versions of Rust, each `HashMap` in `VCodeConstants` occupied at

cranelift/codegen/src/machinst/vcode.rs

@@ -621,6 +621,10 @@ mod test {
 
     #[test]
     fn nop() {
+        // Skip this test when cranelift doesn't support the native platform.
+        if cranelift_native::builder().is_err() {
+            return;
+        }
         let code = String::from(
             "
             test run
@@ -655,6 +659,10 @@ mod test {
 
     #[test]
     fn trampolines() {
+        // Skip this test when cranelift doesn't support the native platform.
+        if cranelift_native::builder().is_err() {
+            return;
+        }
         let function = parse(
             "
             function %test(f32, i8, i64x2, i8) -> f32x4, i64 {

cranelift/filetests/src/function_runner.rs

@@ -34,28 +34,33 @@ fn build_host_isa(
     infer_native_flags: bool,
     flags: settings::Flags,
     isa_flags: Vec<settings::Value>,
-) -> OwnedTargetIsa {
+) -> anyhow::Result<OwnedTargetIsa> {
     let mut builder = cranelift_native::builder_with_options(infer_native_flags)
-        .expect("Unable to build a TargetIsa for the current host");
+        .map_err(|e| anyhow::Error::msg(e))?;
 
     // Copy ISA Flags
     for value in isa_flags {
-        builder.set(value.name, &value.value_string()).unwrap();
+        builder.set(value.name, &value.value_string())?;
     }
 
-    builder.finish(flags).unwrap()
+    let isa = builder.finish(flags)?;
+    Ok(isa)
 }
 
 /// Checks if the host's ISA is compatible with the one requested by the test.
 fn is_isa_compatible(
     file_path: &str,
-    host: &dyn TargetIsa,
+    host: Option<&dyn TargetIsa>,
     requested: &dyn TargetIsa,
 ) -> Result<(), String> {
+    let host_triple = match host {
+        Some(host) => host.triple().clone(),
+        None => target_lexicon::Triple::host(),
+    };
     // If this test requests to run on a completely different
     // architecture than the host platform then we skip it entirely,
     // since we won't be able to natively execute machine code.
-    let host_arch = host.triple().architecture;
+    let host_arch = host_triple.architecture;
     let requested_arch = requested.triple().architecture;
 
     match (host_arch, requested_arch) {
@@ -73,8 +78,8 @@ fn is_isa_compatible(
             | Architecture::Pulley64
             | Architecture::Pulley32be
             | Architecture::Pulley64be,
-        ) if host.triple().pointer_width() == requested.triple().pointer_width()
-            && host.triple().endianness() == requested.triple().endianness() => {}
+        ) if host_triple.pointer_width() == requested.triple().pointer_width()
+            && host_triple.endianness() == requested.triple().endianness() => {}
 
         _ => {
             return Err(format!(
@@ -95,8 +100,15 @@ fn is_isa_compatible(
             Some(requested) => requested,
             None => unimplemented!("ISA flag {} of kind {:?}", req_value.name, req_value.kind()),
         };
-        let available_in_host = host
-            .isa_flags()
+        let host_isa_flags = match host {
+            Some(host) => host.isa_flags(),
+            None => {
+                return Err(format!(
+                    "host not available on this platform for isa-specific flag"
+                ))
+            }
+        };
+        let available_in_host = host_isa_flags
             .iter()
             .find(|val| val.name == req_value.name)
             .and_then(|val| val.as_bool())
@@ -153,7 +165,7 @@ fn compile_testfile(
         // about the operating system / calling convention / etc..
         //
         // Copy the requested ISA flags into the host ISA and use that.
-        _ => build_host_isa(false, flags.clone(), isa.isa_flags()),
+        _ => build_host_isa(false, flags.clone(), isa.isa_flags()).unwrap(),
     };
 
     let mut tfc = TestFileCompiler::new(isa);
@@ -221,8 +233,8 @@ impl SubTest for TestRun {
         }
 
         // Check that the host machine can run this test case (i.e. has all extensions)
-        let host_isa = build_host_isa(true, flags.clone(), vec![]);
-        if let Err(e) = is_isa_compatible(file_path, host_isa.as_ref(), isa.unwrap()) {
+        let host_isa = build_host_isa(true, flags.clone(), vec![]).ok();
+        if let Err(e) = is_isa_compatible(file_path, host_isa.as_deref(), isa.unwrap()) {
             log::info!("{}", e);
             return Ok(());
         }