Stream: git-wasmtime

Topic: wasmtime / PR #12707 s390x-z17: Implement load indexed ad...

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

theotherjimmy opened PR #12707 from theotherjimmy:z17-lxa to bytecodealliance:main:

Starting with Z17, s390x includes some specialized indexed address computation instruction that allow for computing addresses in a way that could map to finding an element within an array, skipping over a structure header of a constant size. While this does not also load the address, it is usually more efficient, at least in terms of code size, to use this newer load indexed address (lxa, llxa) instruction instead of expanded address computations using add instructions.

This is currently created as a draft, as I'm figuring out how to test this. I think testing is needed, as it's a specific sequence of IR instructions, and while I'm confident that I have them correct, it's better to be certain with testing.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 03 2026 at 17:46):

alexcrichton commented on PR #12707:

cc @uweigand

view this post on Zulip Wasmtime GitHub notifications bot (Mar 03 2026 at 19:53):

github-actions[bot] added the label cranelift on PR #12707.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 03 2026 at 19:53):

github-actions[bot] added the label isle on PR #12707.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 03 2026 at 19:54):

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

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "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 (Mar 03 2026 at 20:55):

theotherjimmy updated PR #12707.

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

theotherjimmy updated PR #12707.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 05 2026 at 13:17):

theotherjimmy commented on PR #12707:

Big refactor to avoid expanding the Inst enum. Still needs some runtests, and running said tests.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 05 2026 at 13:42):

theotherjimmy updated PR #12707.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 05 2026 at 13:43):

theotherjimmy commented on PR #12707:

I added rustfmt check to my auto-test script. Sorry I missed that earlier.

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

theotherjimmy updated PR #12707.

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

theotherjimmy commented on PR #12707:

I Wrote tests and verified with a Z17 system.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2026 at 18:53):

theotherjimmy has marked PR #12707 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2026 at 18:53):

theotherjimmy requested cfallin for a review on PR #12707.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2026 at 18:53):

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

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2026 at 19:12):

cfallin commented on PR #12707:

@theotherjimmy I think this will need a rebase; we did a big refactor to add type-parameters to instruction terms in the lowering ISLE environment.

@uweigand would you be willing to review this for correctness? I'm happy to rubber-stamp it (it looks reasonable superficially) once we've got an ISA domain expert approval. Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2026 at 19:21):

theotherjimmy commented on PR #12707:

I'm working on the rebase right now.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 26 2026 at 15:56):

uweigand submitted PR review:

The general approach looks good, but see a few issues inline.

Also, you should add encoding tests for the new instructions to inst/emit_tests.rs.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 26 2026 at 15:56):

uweigand created PR review comment:

This should be an SImm20 to cover the full range of supported displacements.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 26 2026 at 15:56):

uweigand created PR review comment:

I think we need to ensure the inner iadd uses the proper type (I32) - otherwise we might incorrectly catch extends from I16 or I8.

Also, I think we can support a non-zero offset32 field - this can just be passed to the memarg_reg_plus_off like usual (will be used as additional displacement in the actual memory access).

view this post on Zulip Wasmtime GitHub notifications bot (Mar 26 2026 at 15:56):

uweigand created PR review comment:

This does not appear to be used anywhere?

view this post on Zulip Wasmtime GitHub notifications bot (Mar 26 2026 at 15:56):

uweigand created PR review comment:

Here we also need to ensure the correct type of the inner iadd.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 03 2026 at 17:40):

theotherjimmy updated PR #12707.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 03 2026 at 17:41):

theotherjimmy submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 03 2026 at 17:41):

theotherjimmy created PR review comment:

I was forced to resolve this as part of the rebase onto the recent change that added types to clif instructions.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 03 2026 at 17:42):

theotherjimmy commented on PR #12707:

I think I got all the comments and added emit tests.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 03 2026 at 19:37):

alexcrichton requested uweigand for a review on PR #12707.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 07 2026 at 13:52):

uweigand submitted PR review:

LGTM with the two remaining issues fixed. Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 07 2026 at 13:52):

uweigand created PR review comment:

Now this is unused :-)

view this post on Zulip Wasmtime GitHub notifications bot (Apr 07 2026 at 13:52):

uweigand created PR review comment:

This needs to be u64_from_signed_value I think.

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

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

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

theotherjimmy updated PR #12707.

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

theotherjimmy commented on PR #12707:

I resolved the remaining issues.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 09 2026 at 17:04):

uweigand submitted PR review:

LGTM now, thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 09 2026 at 17:05):

uweigand added PR #12707 s390x-z17: Implement load indexed address instruction to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 09 2026 at 17:38):

uweigand merged PR #12707.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 09 2026 at 17:38):

uweigand removed PR #12707 s390x-z17: Implement load indexed address instruction from the merge queue.

Last updated: Apr 12 2026 at 23:10 UTC