Stream: git-wasmtime

Topic: wasmtime / issue #3330 Fix s390x regressions

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2021 at 13:17):

uweigand commented on issue #3330:

Note: In addition to this PR, the endian fix in PR #3329 and the rsix build fix https://github.com/bytecodealliance/rsix/pull/53 are required to make the CI tests fully pass again on s390x.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2021 at 13:21):

uweigand commented on issue #3330:

Also: Using qemu 6.1 we should now be able to turn on upstream CI for the s390x platform.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2021 at 13:33):

github-actions[bot] commented on issue #3330:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "fuzzing"

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

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

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2021 at 16:00):

sunfishcode commented on issue #3330:

I opened https://github.com/bytecodealliance/wasmtime/pull/3331 to pull in the s390x fix in rsix.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2021 at 16:54):

uweigand commented on issue #3330:

I opened #3331 to pull in the s390x fix in rsix.

Thanks, @sunfishcode !

view this post on Zulip Wasmtime GitHub notifications bot (Sep 13 2021 at 15:42):

uweigand commented on issue #3330:

@alexcrichton you were right, I just had to add a single saturating_add. Given that this is just a one-line obvious fix to common code, I've now updated this PR after all. Let me know if you'd prefer me to take it out again.

Note that BranchRI max ranges were just completely wrong, but that type isn't currently used anywhere so it didn't matter. I've now fixed this and used precise max ranges for all types, only capped by a max of 0xffff_ffff due to the u32 type.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 13 2021 at 20:07):

alexcrichton commented on issue #3330:

Nah yeah that's what I would have expected for a saturating_add and it looks good to me! I think someone will still want to look over the sconst changes to the s390x backend though.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 15 2021 at 14:52):

afonso360 commented on issue #3330:

Also: Using qemu 6.1 we should now be able to turn on upstream CI for the s390x platform.

Just want to mention that we have now upgraded to 6.1 in #3325

view this post on Zulip Wasmtime GitHub notifications bot (Sep 15 2021 at 15:27):

uweigand commented on issue #3330:

Nah yeah that's what I would have expected for a saturating_add and it looks good to me! I think someone will still want to look over the sconst changes to the s390x backend though.

Thanks! @cfallin do you want to have a look at those?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 15 2021 at 15:29):

uweigand commented on issue #3330:

Also: Using qemu 6.1 we should now be able to turn on upstream CI for the s390x platform.

Just want to mention that we have now upgraded to 6.1 in #3325

Excellent. Not sure exactly what we need to do to add a CI job for s390x, I haven't done much with GitHub actions before ...

view this post on Zulip Wasmtime GitHub notifications bot (Sep 15 2021 at 15:45):

alexcrichton commented on issue #3330:

To add a CI job theoretically all that needs to be done is to copy/paste this block to add another entry below it, and then update the various values for s390x instead of aarch64.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 15 2021 at 15:49):

uweigand commented on issue #3330:

To add a CI job theoretically all that needs to be done is to copy/paste this block to add another entry below it, and then update the various values for s390x instead of aarch64.

Thanks! I'll give it a try once the two PRs are merged.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2021 at 10:59):

uweigand commented on issue #3330:

So can this be merged now? This is now the final bit blocking successful builds on s390x ...

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2021 at 16:09):

uweigand commented on issue #3330:

To add a CI job theoretically all that needs to be done is to copy/paste this block to add another entry below it, and then update the various values for s390x instead of aarch64.

This is now https://github.com/bytecodealliance/wasmtime/pull/3372 . I also had to disable the lightbeam tests (just like on aarch64).

Last updated: Dec 23 2024 at 12:05 UTC