Stream: git-wasmtime

Topic: wasmtime / PR #12218 s390x: Emit 20bit immedate variants ...

Wasmtime GitHub notifications bot (Dec 24 2025 at 15:30):

theotherjimmy opened PR #12218 from theotherjimmy:s390x-bxd20 to bytecodealliance:main:

Previously, cranelift did not emit memory operations using 20 bit immedates for reg + reg memory modes on s390x. This patch enables that in a type safe manner by exporting a few extractors for 20 bit immediates and changing the argument to the MemArg constructor for reg + reg addressing mode to accept that 20 bit offset instead of the u8 offset.

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

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

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

Wasmtime GitHub notifications bot (Dec 24 2025 at 15:30):

theotherjimmy requested abrown for a review on PR #12218.

Wasmtime GitHub notifications bot (Dec 24 2025 at 15:30):

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

Wasmtime GitHub notifications bot (Dec 24 2025 at 15:32):

theotherjimmy updated PR #12218.

Wasmtime GitHub notifications bot (Dec 24 2025 at 15:52):

theotherjimmy updated PR #12218.

Wasmtime GitHub notifications bot (Dec 24 2025 at 17:45):

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

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:

• cfallin: isle
• fitzgen: isle

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

Learn more.
</details>

Wasmtime GitHub notifications bot (Dec 27 2025 at 04:05):

cfallin commented on PR #12218:

cc @uweigand -- would you mind taking a look?

Wasmtime GitHub notifications bot (Jan 07 2026 at 16:33):

uweigand commented on PR #12218:

Hi @theotherjimmy this makes sense in principle, but I have a couple of comments:

• The bias naming was intended to reflect small offset generated by the back-end to access part of a value (e.g. when accessing a 16-byte value as two 8-byte values, the second value needs to have the address biased by 8); it was not intended to act as a generic displacement. If we want that, we should probably rename the function to reg_plus_reg_plus_off or something.
• With your code, we're now greedily generating reg+reg+simm20 always, and retroactively fix things if the instruction doesn't have that addressing mode. That is probably the right thing to do in general, but in many cases we already know that the addressing mode isn't available (e.g. vector instructions always only have uimm12 displacements). We'll likely get better code if we leave the addition separately from the beginning in those cases.
  

Wasmtime GitHub notifications bot (Jan 07 2026 at 18:14):

theotherjimmy commented on PR #12218:

Thanks for the review @uweigand

I think I'll rework this to write a new extractor with your suggested name, and migrate to that new extractor where we know the instructions support the extended offset. That should resolve your concerns.

Wasmtime GitHub notifications bot (Jan 12 2026 at 19:56):

theotherjimmy updated PR #12218.

Wasmtime GitHub notifications bot (Jan 12 2026 at 19:57):

theotherjimmy updated PR #12218.

Wasmtime GitHub notifications bot (Jan 12 2026 at 19:57):

theotherjimmy commented on PR #12218:

@uweigand Is this more in line with what you expected?

Wasmtime GitHub notifications bot (Jan 13 2026 at 10:19):

uweigand created PR review comment:

This doesn't appear to be used anywhere?

Wasmtime GitHub notifications bot (Jan 13 2026 at 10:19):

uweigand submitted PR review:

This looks generally good to me. As a small enhancement it might be interesting to also extend the lower_address_bias version with a memarg_reg_plus_reg_plus_off clause (which would add the bias to the offset); this probably doesn't have much effect on performance but would preserve the symmetry.

A much bigger (future) enhancement could be to be more specific in generating proper addressing modes for the target instruction in the first place:

• Avoid using the generic RegOffset mode but rather generate explicit code to begin with (e.g. along the lines of the AArch64 amode rules).
• Use more specific flavors of lower_address (e.g. lower_address_bdaddr12, lower_address_bdaddr20, lower_address_bdxaddr12, lower_address_bdxaddr20 to follow LLWM nomenclature) and update the various rules to use the specific flavors instead of the generic, as appropriate for the target instruction of each rule.

Wasmtime GitHub notifications bot (Jan 13 2026 at 16:22):

theotherjimmy updated PR #12218.

Wasmtime GitHub notifications bot (Jan 13 2026 at 17:47):

uweigand submitted PR review.

Wasmtime GitHub notifications bot (Jan 13 2026 at 17:47):

uweigand created PR review comment:

The above rule should now be redundant, shouldn't it? With offset 0 the rule below should always match and give the same result.

Wasmtime GitHub notifications bot (Jan 13 2026 at 18:34):

theotherjimmy updated PR #12218.

Wasmtime GitHub notifications bot (Jan 13 2026 at 18:35):

theotherjimmy submitted PR review.

Wasmtime GitHub notifications bot (Jan 13 2026 at 18:35):

theotherjimmy created PR review comment:

Deleted, tests still pass.

Wasmtime GitHub notifications bot (Jan 13 2026 at 18:52):

uweigand submitted PR review:

This LGTM now, thanks!

Wasmtime GitHub notifications bot (Jan 13 2026 at 19:18):

cfallin submitted PR review:

Thanks Ulrich for the review on this!

Wasmtime GitHub notifications bot (Jan 13 2026 at 19:41):

cfallin merged PR #12218.

Last updated: Feb 24 2026 at 07:22 UTC