Skip to content

Commit

Permalink
Run the full test suite on 32-bit platforms (bytecodealliance#9837)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
alexcrichton authored Jan 15, 2025
1 parent ba950f2 commit 48f4621
Show file tree
Hide file tree
Showing 35 changed files with 445 additions and 287 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
44 changes: 0 additions & 44 deletions ci/build-test-matrix.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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(
{},
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions cranelift/codegen/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
7 changes: 6 additions & 1 deletion cranelift/codegen/src/isa/aarch64/inst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>());
}
}
16 changes: 10 additions & 6 deletions cranelift/codegen/src/isa/pulley_shared/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()],
}
}

Expand Down
6 changes: 3 additions & 3 deletions cranelift/codegen/src/machinst/vcode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions cranelift/filetests/src/function_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
38 changes: 25 additions & 13 deletions cranelift/filetests/src/test_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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!(
Expand All @@ -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())
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(());
}
Expand Down
Loading

0 comments on commit 48f4621

Please sign in to comment.