Skip to content

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

Conversation

saulecabrera
Copy link
Member

@saulecabrera saulecabrera commented Jun 12, 2025

This commit closes #8321

With #11013, Winch passes all spec tests for Core Wasm on Aarch64.

This commit closes
bytecodealliance#8321

With bytecodealliance#11013, Winch
passes all spec tests for Core Wasm on Aarch64.
@saulecabrera saulecabrera requested review from a team as code owners June 12, 2025 19:35
@saulecabrera saulecabrera requested review from alexcrichton and removed request for a team June 12, 2025 19:35
@github-actions github-actions bot added the winch Winch issues or pull requests label Jun 12, 2025
Copy link

Subscribe to Label Action

cc @saulecabrera

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

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

  • saulecabrera: winch

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

Learn more.

@saulecabrera saulecabrera force-pushed the winch-aarch64-enable-integration-tests branch from 99e6487 to 38b8730 Compare June 13, 2025 15:34
@saulecabrera saulecabrera requested a review from a team as a code owner June 13, 2025 15:34
@saulecabrera saulecabrera requested review from fitzgen and removed request for a team June 13, 2025 15:34
`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.
@saulecabrera saulecabrera force-pushed the winch-aarch64-enable-integration-tests branch from 38b8730 to f483cc2 Compare June 13, 2025 15:36
Copy link
Member

@alexcrichton alexcrichton left a 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
@saulecabrera saulecabrera force-pushed the winch-aarch64-enable-integration-tests branch from b4af8a9 to 93bd9d4 Compare June 13, 2025 17:26
@saulecabrera
Copy link
Member Author

@alexcrichton all that make sense, I've pushed the fixes in 93bd9d4

@alexcrichton alexcrichton enabled auto-merge June 13, 2025 17:29
@alexcrichton alexcrichton added this pull request to the merge queue Jun 13, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 13, 2025
@saulecabrera saulecabrera enabled auto-merge June 13, 2025 18:07
@saulecabrera saulecabrera added this pull request to the merge queue Jun 13, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 13, 2025
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.
@saulecabrera saulecabrera added this pull request to the merge queue Jun 13, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 13, 2025
@saulecabrera
Copy link
Member Author

Hmm there seems to be a legit segfault for macOS-arm64, taking a look.

@github-actions github-actions bot added the fuzzing Issues related to our fuzzing infrastructure label Jun 13, 2025
Copy link

Subscribe to Label Action

cc @fitzgen

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

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

  • fitzgen: fuzzing

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

Learn more.

@saulecabrera saulecabrera added this pull request to the merge queue Jun 16, 2025
Merged via the queue into bytecodealliance:main with commit 60269d6 Jun 16, 2025
41 checks passed
@saulecabrera saulecabrera deleted the winch-aarch64-enable-integration-tests branch June 16, 2025 13:58
@saulecabrera
Copy link
Member Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzzing Issues related to our fuzzing infrastructure winch Winch issues or pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

winch(aarch64): MacroAssembler completeness
2 participants