Re: [PATCH v4 13/16] target/riscv: compressed encodings for sspush and sspopchk

[Deepak Gupta <debug@rivosinc.com>]

On Fri, Aug 16, 2024 at 03:09:10PM +1000, Richard Henderson wrote:
>On 8/16/24 11:07, Deepak Gupta wrote:
>>sspush/sspopchk have compressed encodings carved out of zcmops.
>>compressed sspush is designated as c.mop.1 while compressed sspopchk
>>is designated as c.mop.5.
>>
>>Note that c.sspush x1 exists while c.sspush x5 doesn't. Similarly
>>c.sspopchk x5 exists while c.sspopchk x1 doesn't.
>>
>>Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>>Co-developed-by: Jim Shu <jim.shu@sifive.com>
>>Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
>>---
>>  target/riscv/insn16.decode                    |  2 ++
>>  target/riscv/insn_trans/trans_rvzicfiss.c.inc | 12 ++++++++++++
>>  2 files changed, 14 insertions(+)
>>
>>diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
>>index 3953bcf82d..3b84a36233 100644
>>--- a/target/riscv/insn16.decode
>>+++ b/target/riscv/insn16.decode
>>@@ -140,6 +140,8 @@ sw                110  ... ... .. ... 00 @cs_w
>>  addi              000 .  .....  ..... 01 @ci
>>  addi              010 .  .....  ..... 01 @c_li
>>  {
>>+  c_sspush        011 0  00001  00000 01 rs2=1 rs1=0 # c.sspush x1 carving out of zcmops
>>+  c_sspopchk      011 0  00101  00000 01 rs1=5 rd=0 # c.sspopchk x5 carving out of zcmops
>>    c_mop_n         011 0 0 n:3 1 00000 01
>>    illegal         011 0  -----  00000 01 # c.addi16sp and c.lui, RES nzimm=0
>>    addi            011 .  00010  ..... 01 @c_addi16sp
>>diff --git a/target/riscv/insn_trans/trans_rvzicfiss.c.inc b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
>>index 05d439c1f6..67f5c7804a 100644
>>--- a/target/riscv/insn_trans/trans_rvzicfiss.c.inc
>>+++ b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
>>@@ -109,3 +109,15 @@ static bool trans_sspush(DisasContext *ctx, arg_sspush *a)
>>  {
>>      return gen_sspush(ctx, a->rs2);
>>  }
>>+
>>+static bool trans_c_sspopchk(DisasContext *ctx, arg_c_sspopchk *a)
>>+{
>>+    assert(a->rs1 == 5);
>>+    return gen_sspopchk(ctx, a->rs1);
>>+}
>>+
>>+static bool trans_c_sspush(DisasContext *ctx, arg_c_sspush *a)
>>+{
>>+    assert(a->rs2 == 1);
>>+    return gen_sspush(ctx, a->rs2);
>>+}
>
>This indirection is pointless.  Have the decoder invoke the proper 
>insn in the first place.  Identically with how we're treating 'addi', 
>for instance.
>

I was getting compilation error. How to reconcile with arugment sets between
insn32.decode and insn16.decode. Earlier I was doing that and didn't need it.
But after removing indirection in arguments and using single use format, type for
structs instruction arguments ends up conflicting and compiler complains.


>
>r~
>