| Message ID | 20240816010711.3055425-14-debug@rivosinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv support for control flow integrity extensions | expand |
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~
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~ >
On 8/16/24 16:56, Deepak Gupta wrote: >>> + 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 ... >> 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. Ah, for that you need to use named argument sets. In this case, sspush .... &r2_s rs2=1 rs1=0 sspopchk .... &r2 rs2=5 rd=0 in both insn32.decode and insn16.decode r~
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); +}
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(+)