-
Notifications
You must be signed in to change notification settings - Fork 1.5k
winch(aarch64) Run integration tests in winch-aarch64 #11031
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
winch(aarch64) Run integration tests in winch-aarch64 #11031
Conversation
This commit closes bytecodealliance#8321 With bytecodealliance#11013, Winch passes all spec tests for Core Wasm on Aarch64.
Subscribe to Label Action
This issue or pull request has been labeled: "winch"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
99e6487
to
38b8730
Compare
`wast::TestConfig` is shared across spec tests and integration tests, this commits adds a flag to identify if the config is for integration test purposes making it easier to contextually decide if tests are expected to fail or pass depending on the Wasm configuration options.
38b8730
to
f483cc2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it 👍
If you'll indulge a minor bikeshed though, what about inverting this from integration: bool
to spec_test: bool
? I'm a little worried that only the macro-defined tests are integration: true
where all of tests/misc_testsuite/*
should in theory be automatically integration: true
as well. By inverting this to spec_test: bool
it should be easy to audit the single location that classifies spec tests and ensure it sets that and then everything else can ignore it being set.
Also, as a minor nit, instead of adding ..
to patterns can you add spec_test: _
to explicitly ignore the field? The exhaustive match helps ensure that we update the patterns/etc when a new field is added.
To better differentiate spec tests from integration tests
b4af8a9
to
93bd9d4
Compare
@alexcrichton all that make sense, I've pushed the fixes in 93bd9d4 |
Put back `#[wasmtime_test(wasm_features(simd), strategies(not(Winch)))]`; the reason why this tests shouldn't run with Winch is because it requires AVX, but we can have a generic rule at the test config level since not all SIMD based tests require AVX, it really depends on which SIMD instructions are used.
Hmm there seems to be a legit segfault for |
Subscribe to Label Actioncc @fitzgen
This issue or pull request has been labeled: "fuzzing"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
It didn't seem like a segfault in the test itself, nor I was able to reproduce after running the job multiple times (locally and in CI). |
This commit closes #8321
With #11013, Winch passes all spec tests for Core Wasm on Aarch64.