Add AArch64 tests to CI #1526

alexcrichton · 2020-04-16T20:13:53Z

This commit enhances our CI with an AArch64 builder. Currently we have
no physical hardware to run on so for now we run all tests in an
emulator. The AArch64 build is cross-compiled from x86_64 from Linux.
Tests all happen in release mode with a recent version of QEMU (recent
version because it's so much faster, and in release mode because debug
mode tests take quite a long time in an emulator).

The goal here was not to get all tests passing on CI, but rather to get
AArch64 running on CI and get it green at the same time. To achieve that
goal many tests are now ignored on aarch64 platforms. Many tests fail
due to unimplemented functionality in the aarch64 backend (#1521), and
all wasmtime tests involving compilation are also disabled due to
panicking attempting to generate generate instruction offset information
for trap symbolication (#1523).

Despite this, though, all Cranelift tests and other wasmtime tests
should be runnin on AArch64 through QEMU with this PR. Additionally
we'll have an AArch64 binary release of Wasmtime for Linux, although it
won't be too useful just yet since it will panic on almost all wasm
modules.

alexcrichton · 2020-04-16T20:15:19Z

I should note that I tried to write #[should_panic] for all failing tests on aarch64 so if something is fixed we can be sure to enable the regression test. Unfortunately though this directive doesn't work for test functions returning a result, so many wasmtime tests are simply #[ignore]

cranelift/filetests/src/test_run.rs

abrown · 2020-04-16T20:25:42Z

cranelift/filetests/src/test_run.rs

+ let requested_arch = context.isa.unwrap().triple().architecture;
+ if requested_arch != Architecture::host() {
+ return Ok(());
+ }


And we might want to print something saying we skipped a test.

Does the filetest infrastructure print out anything else currently? Curious if I can see an example of what format to print and such.

Over in cranelift/src/filetests/runner.rs it is printing FAIL {path}: {error} so maybe we could print IGNORE {path}: host ISA did not match target ISA? That way at least there is some indication that things didn't run perfectly.

Hm so it looks like the path isn't already threaded through, would you be ok to follow-up with this at a later date?

Sure, tag me in an issue and I will try to look at this later.

Sure thing! Extracted out #1558

github-actions · 2020-04-16T20:26:25Z

Subscribe to Label Action

cc @bnjbvr

This issue or pull request has been labeled: "cranelift"

Thus the following users have been cc'd because of the following labels:

bnjbvr: cranelift

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

github-actions · 2020-04-16T20:33:33Z

Subscribe to Label Action

cc @fitzgen, @kubkon, @peterhuene

This issue or pull request has been labeled: "cranelift:module", "fuzzing", "wasi", "wasmtime:api", "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

fitzgen: fuzzing
kubkon: wasi
peterhuene: wasmtime:api, wasmtime:c-api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

bjorn3 · 2020-04-16T22:04:44Z

ci/build-tarballs.sh

-src=$2
-exe=$3
+src=$1
+exe=$2


This is an internal script we use to build tarballs, it's not really intended to be super-well-documented or make a ton of sense...

Just for future sanity -- maybe standardize on $platform, and add a # Usage: build-tarballs.sh PLATFORM [.exe] (PLATFORM is e.g. x86_64-linux, aarch64-linux) comment (no need for an actual arg check)?

build.rs

alexcrichton · 2020-04-17T20:46:19Z

Updated with a comment about the new semantics of test_run

alexcrichton · 2020-04-20T15:41:07Z

Ok should be green now to account for recent merges.

alexcrichton · 2020-04-21T18:58:11Z

Ok I've pushed up a commit which accounts for #1569 and recently fixed tests too.

This change adds SourceLoc information per instruction in a `VCode<Inst>` container, and keeps this information up-to-date across register allocation and branch reordering. The information is initially collected during instruction lowering, eventually collected on the MachSection, and finally provided to the environment that wraps the codegen crate for wasmtime. This PR is based on top of bytecodealliance#1570 and bytecodealliance#1571 (part of a series fixing tests). This PR depends on wasmtime/regalloc.rs#50, a change to the register allocator to provide instruction-granularity info on the rewritten instruction stream (rather than block-granularity). With the prior PRs applied as well, quite a few more unit tests pass; the exclusion list in bytecodealliance#1526 should be updated if this PR lands first.

This commit enhances our CI with an AArch64 builder. Currently we have no physical hardware to run on so for now we run all tests in an emulator. The AArch64 build is cross-compiled from x86_64 from Linux. Tests all happen in release mode with a recent version of QEMU (recent version because it's so much faster, and in release mode because debug mode tests take quite a long time in an emulator). The goal here was not to get all tests passing on CI, but rather to get AArch64 running on CI and get it green at the same time. To achieve that goal many tests are now ignored on aarch64 platforms. Many tests fail due to unimplemented functionality in the aarch64 backend (bytecodealliance#1521), and all wasmtime tests involving compilation are also disabled due to panicking attempting to generate generate instruction offset information for trap symbolication (bytecodealliance#1523). Despite this, though, all Cranelift tests and other wasmtime tests should be runnin on AArch64 through QEMU with this PR. Additionally we'll have an AArch64 binary release of Wasmtime for Linux, although it won't be too useful just yet since it will panic on almost all wasm modules.

cfallin

LGTM! Just a few comment-addition / clarity requests both otherwise I'm happy with this.

cfallin · 2020-04-22T17:12:48Z

ci/build-tarballs.sh

-src=$2
-exe=$3
+src=$1
+exe=$2


Just for future sanity -- maybe standardize on $platform, and add a # Usage: build-tarballs.sh PLATFORM [.exe] (PLATFORM is e.g. x86_64-linux, aarch64-linux) comment (no need for an actual arg check)?

cfallin · 2020-04-22T17:17:44Z

build.rs

 _ => {}
 },
 _ => panic!("unrecognized strategy"),
 }

 false
 }
+
+fn should_panic(testsuite: &str, testname: &str) -> bool {


Little clarity nit, but maybe add a comment here to distinguish from the "test that this correctly panics" unit-test idiom. Something like // Determine whether to add a should_panic attribute. These tests currently panic because of unfinished backend implementation work; we will remove them from this list as we finish the implementation.

This change adds SourceLoc information per instruction in a `VCode<Inst>` container, and keeps this information up-to-date across register allocation and branch reordering. The information is initially collected during instruction lowering, eventually collected on the MachSection, and finally provided to the environment that wraps the codegen crate for wasmtime. This PR is based on top of bytecodealliance#1570 and bytecodealliance#1571 (part of a series fixing tests). This PR depends on wasmtime/regalloc.rs#50, a change to the register allocator to provide instruction-granularity info on the rewritten instruction stream (rather than block-granularity). With the prior PRs applied as well, quite a few more unit tests pass; the exclusion list in bytecodealliance#1526 should be updated if this PR lands first.

alexcrichton added the cranelift Issues related to the Cranelift code generator label Apr 16, 2020

abrown reviewed Apr 16, 2020

View reviewed changes

cranelift/filetests/src/test_run.rs Show resolved Hide resolved

abrown reviewed Apr 16, 2020

View reviewed changes

github-actions bot added cranelift:module fuzzing Issues related to our fuzzing infrastructure wasi Issues pertaining to WASI wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API. labels Apr 16, 2020

alexcrichton force-pushed the arm64-ci-pr branch from 48ac603 to b376724 Compare April 16, 2020 20:39

bjorn3 reviewed Apr 16, 2020

View reviewed changes

github-actions bot added the cranelift:docs label Apr 17, 2020

alexcrichton force-pushed the arm64-ci-pr branch from 13d04ad to f428b51 Compare April 17, 2020 22:25

cfallin added the cranelift:area:aarch64 Issues related to AArch64 backend. label Apr 18, 2020

alexcrichton force-pushed the arm64-ci-pr branch from f428b51 to b9c9a55 Compare April 20, 2020 15:09

alexcrichton mentioned this pull request Apr 20, 2020

Cranelift test_run command doesn't print skipped tests #1558

Closed

alexcrichton force-pushed the arm64-ci-pr branch from b9c9a55 to ca495dc Compare April 21, 2020 18:57

cfallin mentioned this pull request Apr 22, 2020

MachInst backend: pass through SourceLoc information. #1575

Merged

alexcrichton force-pushed the arm64-ci-pr branch from ca495dc to eb77fb7 Compare April 22, 2020 15:05

alexcrichton force-pushed the arm64-ci-pr branch from eb77fb7 to 95e3014 Compare April 22, 2020 15:24

cfallin approved these changes Apr 22, 2020

View reviewed changes

Review comments

fec9653

alexcrichton merged commit d1aa86f into bytecodealliance:master Apr 22, 2020

alexcrichton deleted the arm64-ci-pr branch April 22, 2020 17:57

alexcrichton mentioned this pull request Apr 22, 2020

Sync expected successful tests on AArch64 #1581

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AArch64 tests to CI #1526

Add AArch64 tests to CI #1526

alexcrichton commented Apr 16, 2020

alexcrichton commented Apr 16, 2020

abrown Apr 16, 2020

alexcrichton Apr 16, 2020

abrown Apr 16, 2020

alexcrichton Apr 17, 2020

abrown Apr 17, 2020

alexcrichton Apr 20, 2020

github-actions bot commented Apr 16, 2020

github-actions bot commented Apr 16, 2020

bjorn3 Apr 16, 2020

alexcrichton Apr 17, 2020

cfallin Apr 22, 2020

alexcrichton commented Apr 17, 2020

alexcrichton commented Apr 20, 2020

alexcrichton commented Apr 21, 2020

cfallin left a comment

cfallin Apr 22, 2020

cfallin Apr 22, 2020

Add AArch64 tests to CI #1526

Add AArch64 tests to CI #1526

Conversation

alexcrichton commented Apr 16, 2020

alexcrichton commented Apr 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 16, 2020

Subscribe to Label Action

github-actions bot commented Apr 16, 2020

Subscribe to Label Action

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcrichton commented Apr 17, 2020

alexcrichton commented Apr 20, 2020

alexcrichton commented Apr 21, 2020

cfallin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment