afonso360 opened PR #7057 from afonso360:riscv-c-inst-part-2
to bytecodealliance:main
:
:wave: Hey,
This PR introduces all of the compressed variations of the
addi
instruction (4 different adds!) as well as compressed trap, compresseddebugtrap
, and compressed shift left.We have the following add variants:
c.addi
this is the normaladdi
, with a signed 6 bit immediate field.c.addiw
also fairly normal, a 32 bit add with a 6 bit immediate, that sign extends the result to 64bits.c.addi16sp
modifies the stack pointer with a signed 6 bit immediate scaled by 16.c.addi4spn
adds a zero extended 6 bit immediate scaled by 4 to the stack pointer. Unlikec.addi16sp
it allows saving the result in any compressible register.
afonso360 requested abrown for a review on PR #7057.
afonso360 requested wasmtime-compiler-reviewers for a review on PR #7057.
alexcrichton submitted PR review.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
To avoid the double-construction logic for
Imm6::maybe_from_imm12
could this do:let imm6 = match Imm6::maybe_from_imm12(imm12) { Some(imm6) => imm6, None => return false, };
or alternatively if this function returned
Option<()>
instead ofbool
it could use?
. Or alternatively I think this could uselet Some(imm6) = ... else { return false; };
, but I haven't uselet
/else
much myself.
alexcrichton created PR review comment:
Instead of gating all these rules with
has_zca
, could that be at the top of the function to early-returnfalse
if it's not enabled?
alexcrichton created PR review comment:
Should this be
c.addiw
instead ofc.sext.w
?
afonso360 submitted PR review.
afonso360 created PR review comment:
Yes, I think we can. I was worried that it was possible for a target to implement
Zcd
orZcb
(which I'm planing on implementing soon) withoutZca
, but I looked at the spec and both of them requireZca
to be present.
afonso360 submitted PR review.
afonso360 created PR review comment:
Oops, yes it should.
alexcrichton submitted PR review:
Oh I meant to do this too
afonso360 submitted PR review.
afonso360 created PR review comment:
Here I think it's ok to do that, but I'd like to keep the separate check for
c.addi16sp
andc.addi4spn
so that we can try to match all 3 instructions. Otherwise we could enter into one branch and never emit the instruction.
afonso360 edited PR review comment.
afonso360 edited PR review comment.
afonso360 updated PR #7057.
afonso360 has enabled auto merge for PR #7057.
afonso360 merged PR #7057.
Last updated: Dec 23 2024 at 13:07 UTC