Skip to content

pulley: Add more addressing modes for loads/stores#9994

Merged
alexcrichton merged 2 commits intobytecodealliance:mainfrom
alexcrichton:pulley-wasm-addressing-modes
Jan 13, 2025
Merged

pulley: Add more addressing modes for loads/stores#9994
alexcrichton merged 2 commits intobytecodealliance:mainfrom
alexcrichton:pulley-wasm-addressing-modes

Conversation

@alexcrichton
Copy link
Member

This commit adds a new "g32" addressing mode to Pulley that matches the pattern emitted by Cranelift for 32-bit wasm guests running on hosts. The general idea here is that this addressing mode encompasses an addition of a host-width value to a zero-extended (optionally) 32-bit value. On 32-bit hosts there's no zero-extension but on 64-bit hosts there's a zero-extension. The wasm address is always 32-bits though which enables using a single instruction for both 32 and 64-bit hosts.

New "g32" loads and stores are added to Pulley with varying sizes and options according to what seems to be common in wasm. The disas test suite was updated to showcase using these instructions for wasm loads/stores on 32 and 64-bit hosts.

An additional change in this commit is to deduplicate the 32/64-bit bounds-check macro-ops. The trick in this commit works for those as well meaning that only a single instruction is needed instead of one-per-host-pointer-width. Additionally the load of the bound from the VMContext is folded into the bounds check itself as it was found that this was always present anyway before the bounds check.

Overall this shrinks the size of spidermonkey.cwasm from 21M to 20M and the runtime of pulldown-cmark, bz2, and spidermonkey on Sightglass have all been reduced by 10%. Not as big wins as I was hoping for but alas.

@alexcrichton alexcrichton requested review from a team as code owners January 13, 2025 16:58
@alexcrichton alexcrichton requested review from cfallin and fitzgen and removed request for a team January 13, 2025 16:58
@alexcrichton alexcrichton mentioned this pull request Jan 13, 2025
13 tasks
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator isle Related to the ISLE domain-specific language pulley Issues related to the Pulley interpreter labels Jan 13, 2025
@github-actions
Copy link

Subscribe to Label Action

cc @cfallin, @fitzgen

Details This issue or pull request has been labeled: "cranelift", "isle", "pulley"

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

  • cfallin: isle
  • fitzgen: isle, pulley

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

Learn more.

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@fitzgen fitzgen added this pull request to the merge queue Jan 13, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 13, 2025
@alexcrichton alexcrichton added this pull request to the merge queue Jan 13, 2025
This commit adds a new "g32" addressing mode to Pulley that matches the
pattern emitted by Cranelift for 32-bit wasm guests running on hosts.
The general idea here is that this addressing mode encompasses an
addition of a host-width value to a zero-extended (optionally) 32-bit
value. On 32-bit hosts there's no zero-extension but on 64-bit hosts
there's a zero-extension. The wasm address is always 32-bits though
which enables using a single instruction for both 32 and 64-bit hosts.

New "g32" loads and stores are added to Pulley with varying sizes and
options according to what seems to be common in wasm. The `disas` test
suite was updated to showcase using these instructions for wasm
loads/stores on 32 and 64-bit hosts.

An additional change in this commit is to deduplicate the 32/64-bit
bounds-check macro-ops. The trick in this commit works for those as well
meaning that only a single instruction is needed instead of
one-per-host-pointer-width. Additionally the load of the bound from the
`VMContext` is folded into the bounds check itself as it was found that
this was always present anyway before the bounds check.

Overall this shrinks the size of `spidermonkey.cwasm` from 21M to 20M
and the runtime of `pulldown-cmark`, `bz2`, and `spidermonkey` on
Sightglass have all been reduced by 10%. Not as big wins as I was hoping
for but alas.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jan 13, 2025
@alexcrichton alexcrichton force-pushed the pulley-wasm-addressing-modes branch from 68a8f66 to e56e6a8 Compare January 13, 2025 20:54
@alexcrichton alexcrichton added this pull request to the merge queue Jan 13, 2025
Merged via the queue into bytecodealliance:main with commit a6a0857 Jan 13, 2025
37 checks passed
@alexcrichton alexcrichton deleted the pulley-wasm-addressing-modes branch January 13, 2025 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift Issues related to the Cranelift code generator isle Related to the ISLE domain-specific language pulley Issues related to the Pulley interpreter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants