Stream: git-wasmtime

Topic: wasmtime / PR #12319 S390x: emit new instructions added i...


view this post on Zulip Wasmtime GitHub notifications bot (Jan 12 2026 at 15:57):

theotherjimmy opened PR #12319 from theotherjimmy:s390x-z17 to bytecodealliance:main:

Z17 (arch15) includes some instructions that allow us to encode some more complicated operations in fewer instructions. This PR adds support to cranelift-codegen to emit these newer instructions when appropriate.

Further, Z17 includes a VBLEND instruction that mimics the same instruction on x64. Since this is no longer an x64-exclusive instruction type, I've renamed the appropriate stuff within cranelift codegen to reflect that this is not ISA-specific anymore.

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Jan 12 2026 at 15:57):

theotherjimmy requested alexcrichton for a review on PR #12319.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 12 2026 at 15:57):

theotherjimmy requested wasmtime-compiler-reviewers for a review on PR #12319.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 12 2026 at 16:02):

theotherjimmy updated PR #12319.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 12 2026 at 16:20):

theotherjimmy updated PR #12319.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 12 2026 at 16:26):

theotherjimmy updated PR #12319.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 12 2026 at 17:47):

github-actions[bot] commented on PR #12319:

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:x64", "cranelift:meta", "isle"

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 (Jan 12 2026 at 20:58):

alexcrichton commented on PR #12319:

I'm going to shift review of this over to @uweigand

view this post on Zulip Wasmtime GitHub notifications bot (Jan 12 2026 at 20:59):

alexcrichton commented on PR #12319:

or, well, I can't officially do that, but @uweigand I'm happy to rubber-stamp once you've approved

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 15:16):

uweigand submitted PR review:

Looks mostly good to me, but see inline comments.

In addition to what is implemented here, we now could implement vector integer division for 32-bit and 64-bit integer vectors - but there is currently no ISLE to even express this.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 15:16):

uweigand created PR review comment:

Not sure what if any canonicalization is done at the ISLE level here, but these four don't cover all possible combinations. E.g. (band x (band (bnot y) z)) is not covered.

I guess a more fundamental question is which combinations we should be covering. For example, why cover and-not with three inputs but not or-not?

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 15:16):

uweigand created PR review comment:

We should also test the encoding for VBLEND and VEVAL here.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 15:16):

uweigand created PR review comment:

We should also add z17 insns to the disassembler, but that is of course a different patch.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 15:16):

uweigand created PR review comment:

I think we need to fix this before committing. (Also, there probably should be run-time test validating the correct behaviour like for other divison operations.)

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 15:16):

uweigand created PR review comment:

Should this function also be renamed to lose the x86 then?

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 16:15):

theotherjimmy submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 16:15):

theotherjimmy created PR review comment:

I stopped because I realized this would add hundreds of rules to get correct, and ran out of steam pretty quickly. I think it's an open question: what should we encode? what 3-input binary operations are actually used?

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 18:26):

theotherjimmy updated PR #12319.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 18:26):

theotherjimmy submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 18:26):

theotherjimmy created PR review comment:

Done.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 18:27):

theotherjimmy submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 13 2026 at 18:27):

theotherjimmy created PR review comment:

Added tests.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2026 at 21:00):

theotherjimmy updated PR #12319.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2026 at 21:04):

theotherjimmy submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2026 at 21:04):

theotherjimmy created PR review comment:

I made a python script to handle this, and committed the results. My thinking is this:

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2026 at 21:05):

theotherjimmy submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2026 at 21:05):

theotherjimmy created PR review comment:

Is the integer overflow we're checking for diving INTMIN by -1?

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2026 at 21:45):

theotherjimmy submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2026 at 21:45):

theotherjimmy created PR review comment:

That's a yes. I missed that bit of the instruction description.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2026 at 16:10):

theotherjimmy updated PR #12319.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2026 at 16:11):

theotherjimmy submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2026 at 16:11):

theotherjimmy created PR review comment:

Fixed.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 28 2026 at 14:52):

uweigand submitted PR review:

This looks all good to me now, except for the srem implementation - see inline comment.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 28 2026 at 14:52):

uweigand created PR review comment:

The sdiv case looks good now, but the srem case is wrong - in the case of division of INT_MIN by -1, only the quotient overflows, not the remainder. The remainder in this case is simply zero, just as with any division by -1. See the comment above and the maybe_avoid_srem_overflow helper. I think this can be implemented following the same logic for $I128 as well.


Last updated: Jan 29 2026 at 13:25 UTC