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

[Richard Henderson <richard.henderson@linaro.org>]

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.


r~