Stream: git-wasmtime

Topic: wasmtime / PR #6366 riscv64: Use Vector RegClass for Vectors


view this post on Zulip Wasmtime GitHub notifications bot (May 10 2023 at 10:05):

afonso360 opened PR #6366 from afonso360:riscv-regclass-vec to bytecodealliance:main:

:wave: Hey,

This PR changes the RISC-V backend to use the new Vector Regclass when dealing with vectors. It also implements the calling convention described in this document. Essentially all vector arguments are passed via stack.

Additionally this also adds a test for the vconst pool, which was supposed to be merged in with #6324 but was failing due to some missing ABI stuff.

I've also had to limit the maximum usable SIMD type down to 1024bits, it looks like regalloc2 does not support spills larger than 2040 bytes and it's not really worth fixing it. 1024bit vectors ought to be enough for anybody.

view this post on Zulip Wasmtime GitHub notifications bot (May 10 2023 at 10:05):

afonso360 requested cfallin for a review on PR #6366.

view this post on Zulip Wasmtime GitHub notifications bot (May 10 2023 at 10:05):

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

view this post on Zulip Wasmtime GitHub notifications bot (May 10 2023 at 10:08):

afonso360 created PR review comment:

I'm not 100% sure about this. v0 is a completely normal vector register, however it is also the only vector register that can be used as a mask with instructions. i.e. when instructions are masked they implicitly read v0.

Should I mark v0 as non preferable to avoid unnecessary moves, or will regalloc figure it out once we start adding fixed_def's?

To be clear, masks can be computed in any vector register, they can only be used in v0.

view this post on Zulip Wasmtime GitHub notifications bot (May 10 2023 at 10:08):

afonso360 edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (May 10 2023 at 10:11):

afonso360 edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (May 10 2023 at 10:13):

afonso360 edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (May 10 2023 at 10:13):

afonso360 edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (May 10 2023 at 13:54):

afonso360 edited PR #6366:

:wave: Hey,

This PR changes the RISC-V backend to use the new Vector Regclass when dealing with vectors. It also implements the calling convention described in this document. Essentially all vector arguments are passed via stack and all registers are caller saved.

Additionally this also adds a test for the vconst pool, which was supposed to be merged in with #6324 but was failing due to some missing ABI stuff.

I've also had to limit the maximum usable SIMD type down to 1024bits, it looks like regalloc2 does not support spills larger than 2040 bytes and it's not really worth fixing it. 1024bit vectors ought to be enough for anybody.

view this post on Zulip Wasmtime GitHub notifications bot (May 15 2023 at 20:22):

cfallin submitted PR review:

Looks mostly good to me, thanks!

Just one possible improvement below but if you'd rather merge as-is and tackle it in a followup or leave as a TODO, I think that's fine too.

view this post on Zulip Wasmtime GitHub notifications bot (May 15 2023 at 20:22):

cfallin submitted PR review:

Looks mostly good to me, thanks!

Just one possible improvement below but if you'd rather merge as-is and tackle it in a followup or leave as a TODO, I think that's fine too.

view this post on Zulip Wasmtime GitHub notifications bot (May 15 2023 at 20:22):

cfallin created PR review comment:

This implementation looks correct, but FWIW the hope at least with the PRegSet definition was to allow for constants (they're small bitsets, u128 or now u256) to be returned by backends. I realize the existing backend also doesn't do this, so in the worst case we can keep the status quo. But this might even be a small but visible performance issue for function bodies with frequent calls. Would it be possible to define a const fn that computes a PRegSet here (either from existing constants or with a straight-line "add reg ..." sequence)?

view this post on Zulip Wasmtime GitHub notifications bot (May 15 2023 at 20:22):

cfallin created PR review comment:

I think it's fine to have all vec-regs be equally preferred; if an instruction constrains an arg to require v0 then RA2 will do the right thing and put other values in other registers as long as they're available (or split/spill otherwise). The preferred/non-preferred mechanism is mainly there to push allocations toward caller-saves (volatiles) and avoid prologue/epilogue instructions where possible.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 18:38):

afonso360 updated PR #6366.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 18:40):

afonso360 has enabled auto merge for PR #6366.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:49):

afonso360 merged PR #6366.


Last updated: Nov 22 2024 at 16:03 UTC