Stream: git-wasmtime

Topic: wasmtime / PR #8703 riscv64: Improve base select lowering

view this post on Zulip Wasmtime GitHub notifications bot (May 29 2024 at 13:55):

afonso360 opened PR #8703 from afonso360:riscv-better-select to bytecodealliance:main:

:wave: Hey,

This PR is an attempt at improving the base lowering for the select instruction in the RISC-V backend. It reduces one instruction from the current lowering.

Currently our selects have the following instructions:

b{cond} rc, 0xc
mv rd, rx
j 8
mv rd, ry

With this change we now produce the following:

mv rd, rx
b{cond} rc, 8
mv rd, ry

So, we lose the jump instruction, but one of the moves becomes unconditional. This to me feels like it's worth it, but I have no data to prove one way or the other, and I think if i try to measure it with sightglass it probably won't detect this change.

One of the benefits of this lowering is that now if one of the input registers is the same as the destination register we can do a two instruction select, like so (assuming rx = rd):

b{cond} rc, 8
mv rd, ry

I've had to do some regalloc surgery to make that possible which makes some other cases get worse regalloc.

This is built on top of #8695, but only the last commit is relevant.

view this post on Zulip Wasmtime GitHub notifications bot (May 29 2024 at 13:55):

afonso360 requested abrown for a review on PR #8703.

view this post on Zulip Wasmtime GitHub notifications bot (May 29 2024 at 13:55):

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

view this post on Zulip Wasmtime GitHub notifications bot (May 29 2024 at 13:55):

afonso360 requested wasmtime-default-reviewers for a review on PR #8703.

view this post on Zulip Wasmtime GitHub notifications bot (May 29 2024 at 13:59):

afonso360 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 29 2024 at 13:59):

afonso360 created PR review comment:

We now sometimes get this funky output from regalloc. I think I understand why it's doing this, but maybe there's some way to avoid this?

As a side note, maybe we could force the destination register to always be the same as one of the inputs. We would always emit the 2 instruction select and let regalloc emit the move beforehand if it finds it beneficial.

I'm not sure if this is better or not.

view this post on Zulip Wasmtime GitHub notifications bot (May 29 2024 at 16:44):

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

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:riscv64", "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 (May 29 2024 at 16:45):

abrown commented on PR #8703:

I'm positive toward this change but someone more familiar with RISC-V should probably review this?

view this post on Zulip Wasmtime GitHub notifications bot (May 29 2024 at 17:11):

cfallin submitted PR review:

Last commit LGTM, thanks! Happy to land once #8695 does and this rebases.

view this post on Zulip Wasmtime GitHub notifications bot (May 31 2024 at 16:35):

afonso360 updated PR #8703.

view this post on Zulip Wasmtime GitHub notifications bot (May 31 2024 at 16:36):

afonso360 has enabled auto merge for PR #8703.

view this post on Zulip Wasmtime GitHub notifications bot (May 31 2024 at 17:00):

afonso360 merged PR #8703.

Last updated: Nov 22 2024 at 16:03 UTC