Stream: git-wasmtime

Topic: wasmtime / PR #7145 riscv64: Consolidate conditional move...

view this post on Zulip Wasmtime GitHub notifications bot (Oct 03 2023 at 21:39):

alexcrichton opened PR #7145 from alexcrichton:rv64-refactor-select-instrs to bytecodealliance:main:

This commit removes the IntSelect and SelectReg pseudo-instructions
from the riscv64 backend and consolidates them into the Select
instruction. Additionally the Select instruction is updated to subsume
the functionality of these two previous instructions. Namely Select
now operates with ValueRegs to handle i128 and additionally takes an
IntegerCompare as the condition for the conditional branch to use.

This commit touches a fair bit of the backend since conditional
selection of registers was used in quite a few places. The previous
gen_select_* functions are replaced with new typed equivalents of
gen_select_{xreg,vreg,freg,regs}. Furthermore new cmp_* helpers were
added to create IntegerCompare instructions which sort-of match
conditional branch instructions, or at least the pnemonics they use.

Finally since this affected the select CLIF instruction itself I went
ahead and did some refactoring there too. The select instruction
creates an IntegerCompare from its argument to use to generate the
appropriate register selection instruction. This is basically the same
thing that brif does and now both go through a new helper,
lower_int_compare, which takes a Value and produces an
IntegerCompare representing if that value is either true or false.
This enables folding an icmp or an fcmp, for example, directly into
a branching instruction.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 03 2023 at 21:39):

alexcrichton requested afonso360 for a review on PR #7145.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 03 2023 at 21:39):

alexcrichton requested abrown for a review on PR #7145.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 03 2023 at 21:39):

alexcrichton requested wasmtime-compiler-reviewers for a review on PR #7145.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 01:16):

alexcrichton updated PR #7145.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 09:30):

afonso360 submitted PR review:

This looks great to me! Thanks! A small suggestion below.

I also noticed that we are missing support for the {u,s}{min,max} instructions that I thought were implemented, but apparently not. I'll submit a follow up PR to this one adding them.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 09:30):

afonso360 submitted PR review:

This looks great to me! Thanks! A small suggestion below.

I also noticed that we are missing support for the {u,s}{min,max} instructions that I thought were implemented, but apparently not. I'll submit a follow up PR to this one adding them.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 09:30):

afonso360 created PR review comment:

Another small optimization here is that we could sign extend IntCC::Equal and IntCC::NotEqual for 32bit values. To take advantage of the same sign extend instruction as above.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 14:40):

alexcrichton updated PR #7145.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 14:41):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 14:41):

alexcrichton created PR review comment:

Excellent point!

I swear I'm eventually getting back towards icmp (slowly!), and I thought this was a neat optimization in your original PR for icmp. I've added it here as well

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 14:41):

alexcrichton has enabled auto merge for PR #7145.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 14:48):

afonso360 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 14:48):

afonso360 created PR review comment:

Even without icmp these refactors are very much appreciated!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 16:23):

alexcrichton updated PR #7145.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 16:23):

alexcrichton has enabled auto merge for PR #7145.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 04 2023 at 20:44):

alexcrichton merged PR #7145.

Last updated: Dec 23 2024 at 12:05 UTC