Stream: git-wasmtime

Topic: wasmtime / PR #7079 riscv64: Better AMode Matching


view this post on Zulip Wasmtime GitHub notifications bot (Sep 23 2023 at 17:09):

afonso360 opened PR #7079 from afonso360:riscv-sp-load-store to bytecodealliance:main:

:wave: Hey,

This PR improves our AMode matching logic. In RISC-V we don't have very fancy addressing modes, it's pretty much Reg + Offset.

One of the issues with the current lowerings is that we never feed the sp register into a load or store, even if it is based on a stack_addr. This is because the regular stack_addr lowering moves the address into a normal register. We now directly match the stack_addr as part of the amode computation so that we can perform those loads directly from the stack pointer and without an intermediary register.

Another improvement that this PR makes is that we now match an iadd+iconst and move that into the offset field for the load or store.

The main motivation for this is that we have some stack pointer based load and store instructions in the C extension that very rarely match because we never do loads or stores directly based on the stack pointer.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 23 2023 at 17:09):

afonso360 requested fitzgen for a review on PR #7079.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 23 2023 at 17:09):

afonso360 requested wasmtime-compiler-reviewers for a review on PR #7079.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 24 2023 at 06:00):

alexcrichton submitted PR review:

Looks good to me :+1:

One suggestion I might have, orthogonal to this PR, is to move towards pushing the LoadAddr emit logic into ISLE itself. In theory it should be possible to push most of that into the amode_inner constructor you've added here and, ideally, remove LoadAddr entirely. Personally I've found pushing logic into ISLE as "here's all the tricky architecture-specific bits" works pretty well, although I realize much of this code here likely predates ISLE so this is mostly just a suggestion on a possible direction for the future.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 24 2023 at 06:50):

alexcrichton merged PR #7079.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2023 at 19:39):

fitzgen submitted PR review:

Nice!


Last updated: Nov 22 2024 at 16:03 UTC