* [PATCH] Add RISCV Zilsd extension @ 2025-11-04 11:59 Roan Richmond 2025-11-06 12:17 ` Daniel Henrique Barboza 0 siblings, 1 reply; 5+ messages in thread From: Roan Richmond @ 2025-11-04 11:59 UTC (permalink / raw) To: qemu-riscv Cc: palmer, alistair.francis, liwei1518, dbarboza, zhiwei_liu, qemu-devel, alistair23, Roan Richmond This is based on v1.0 of the Zilsd extension [1]. The specification is listed as in the Ratified state [2], since Jan 30, 2025. [1]: https://github.com/riscv/riscv-zilsd [2]: https://riscv.atlassian.net/wiki/spaces/HOME/pages/16154861/RISC-V+Specs+Under+Development Signed-off-by: Roan Richmond <roan.richmond@codethink.co.uk> --- target/riscv/cpu.c | 2 + target/riscv/cpu_cfg_fields.h.inc | 1 + target/riscv/insn_trans/trans_rvi.c.inc | 56 ++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 73d4280d7c..6e0d37fb96 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -121,6 +121,7 @@ const RISCVIsaExtData isa_edata_arr[] = { ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl), ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm), + ISA_EXT_DATA_ENTRY(zilsd, PRIV_VERSION_1_13_0, ext_zilsd), ISA_EXT_DATA_ENTRY(zimop, PRIV_VERSION_1_13_0, ext_zimop), ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul), ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, has_priv_1_12), @@ -1247,6 +1248,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = { MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true), MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true), MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true), + MULTI_EXT_CFG_BOOL("zilsd", ext_zilsd, false), MULTI_EXT_CFG_BOOL("zimop", ext_zimop, false), MULTI_EXT_CFG_BOOL("zcmop", ext_zcmop, false), MULTI_EXT_CFG_BOOL("zacas", ext_zacas, false), diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc index a154ecdc79..8d8e35e4cf 100644 --- a/target/riscv/cpu_cfg_fields.h.inc +++ b/target/riscv/cpu_cfg_fields.h.inc @@ -41,6 +41,7 @@ BOOL_FIELD(ext_zicond) BOOL_FIELD(ext_zihintntl) BOOL_FIELD(ext_zihintpause) BOOL_FIELD(ext_zihpm) +BOOL_FIELD(ext_zilsd) BOOL_FIELD(ext_zimop) BOOL_FIELD(ext_zcmop) BOOL_FIELD(ext_ztso) diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc index 54b9b4f241..dcbb3811d9 100644 --- a/target/riscv/insn_trans/trans_rvi.c.inc +++ b/target/riscv/insn_trans/trans_rvi.c.inc @@ -370,6 +370,27 @@ static bool gen_load_tl(DisasContext *ctx, arg_lb *a, MemOp memop) return true; } +/* Zilsd extension adds load/store double for 32bit arch */ +static bool gen_load_zilsd(DisasContext *ctx, arg_lb *a) +{ + TCGv dest_1 = dest_gpr(ctx, a->rd); + TCGv dest_2 = dest_gpr(ctx, (a->rd)+1); + TCGv addr_1 = get_address(ctx, a->rs1, a->imm); + TCGv addr_2 = get_address(ctx, a->rs1, (a->imm)+4); + + tcg_gen_qemu_ld_tl(dest_1, addr_1, ctx->mem_idx, MO_SL); + tcg_gen_qemu_ld_tl(dest_2, addr_2, ctx->mem_idx, MO_SL); + + /* If destination is x0 then result of the load is discarded */ + if (a->rd == 0) { + return true; + } + + gen_set_gpr(ctx, a->rd, dest_1); + gen_set_gpr(ctx, a->rd+1, dest_2); + return true; +} + /* Compute only 64-bit addresses to use the address translation mechanism */ static bool gen_load_i128(DisasContext *ctx, arg_lb *a, MemOp memop) { @@ -409,6 +430,8 @@ static bool gen_load(DisasContext *ctx, arg_lb *a, MemOp memop) decode_save_opc(ctx, 0); if (get_xl(ctx) == MXL_RV128) { out = gen_load_i128(ctx, a, memop); + } else if (memop == MO_SQ && get_xl(ctx) == MXL_RV32) { + out = gen_load_zilsd(ctx, a); } else { out = gen_load_tl(ctx, a, memop); } @@ -437,7 +460,10 @@ static bool trans_lw(DisasContext *ctx, arg_lw *a) static bool trans_ld(DisasContext *ctx, arg_ld *a) { - REQUIRE_64_OR_128BIT(ctx); + /* Check for Zilsd extension if 32bit */ + if (get_xl(ctx) == MXL_RV32 && !ctx->cfg_ptr->ext_zilsd) { + return false; + } return gen_load(ctx, a, MO_SQ); } @@ -482,6 +508,27 @@ static bool gen_store_tl(DisasContext *ctx, arg_sb *a, MemOp memop) return true; } +/* Zilsd extension adds load/store double for 32bit arch */ +static bool gen_store_zilsd(DisasContext *ctx, arg_sb *a) +{ + TCGv data_1 = tcg_temp_new(); + TCGv data_2 = tcg_temp_new(); + if (a->rs2 != 0) { + data_1 = get_gpr(ctx, a->rs2, EXT_NONE); + data_2 = get_gpr(ctx, (a->rs2)+1, EXT_NONE); + } + TCGv addr_1 = get_address(ctx, a->rs1, a->imm); + TCGv addr_2 = get_address(ctx, a->rs1, (a->imm)+4); + + if (ctx->ztso) { + tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); + } + + tcg_gen_qemu_st_tl(data_1, addr_1, ctx->mem_idx, MO_SL); + tcg_gen_qemu_st_tl(data_2, addr_2, ctx->mem_idx, MO_SL); + return true; +} + static bool gen_store_i128(DisasContext *ctx, arg_sb *a, MemOp memop) { TCGv src1l = get_gpr(ctx, a->rs1, EXT_NONE); @@ -511,6 +558,8 @@ static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop) decode_save_opc(ctx, 0); if (get_xl(ctx) == MXL_RV128) { return gen_store_i128(ctx, a, memop); + } else if (memop == MO_UQ && get_xl(ctx) == MXL_RV32) { + return gen_store_zilsd(ctx, a); } else { return gen_store_tl(ctx, a, memop); } @@ -533,7 +582,10 @@ static bool trans_sw(DisasContext *ctx, arg_sw *a) static bool trans_sd(DisasContext *ctx, arg_sd *a) { - REQUIRE_64_OR_128BIT(ctx); + /* Check for Zilsd extension if 32bit */ + if (get_xl(ctx) == MXL_RV32 && !ctx->cfg_ptr->ext_zilsd) { + return false; + } return gen_store(ctx, a, MO_UQ); } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Add RISCV Zilsd extension 2025-11-04 11:59 [PATCH] Add RISCV Zilsd extension Roan Richmond @ 2025-11-06 12:17 ` Daniel Henrique Barboza 2025-11-06 14:31 ` Roan Richmond 0 siblings, 1 reply; 5+ messages in thread From: Daniel Henrique Barboza @ 2025-11-06 12:17 UTC (permalink / raw) To: Roan Richmond, qemu-riscv Cc: palmer, alistair.francis, liwei1518, zhiwei_liu, qemu-devel, alistair23 On 11/4/25 8:59 AM, Roan Richmond wrote: > This is based on v1.0 of the Zilsd extension [1]. > The specification is listed as in the Ratified state [2], > since Jan 30, 2025. > > [1]: https://github.com/riscv/riscv-zilsd > [2]: https://riscv.atlassian.net/wiki/spaces/HOME/pages/16154861/RISC-V+Specs+Under+Development > > Signed-off-by: Roan Richmond <roan.richmond@codethink.co.uk> > --- > target/riscv/cpu.c | 2 + > target/riscv/cpu_cfg_fields.h.inc | 1 + > target/riscv/insn_trans/trans_rvi.c.inc | 56 ++++++++++++++++++++++++- > 3 files changed, 57 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 73d4280d7c..6e0d37fb96 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -121,6 +121,7 @@ const RISCVIsaExtData isa_edata_arr[] = { > ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl), > ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm), > + ISA_EXT_DATA_ENTRY(zilsd, PRIV_VERSION_1_13_0, ext_zilsd), > ISA_EXT_DATA_ENTRY(zimop, PRIV_VERSION_1_13_0, ext_zimop), > ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul), > ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, has_priv_1_12), > @@ -1247,6 +1248,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = { > MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true), > MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true), > MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true), > + MULTI_EXT_CFG_BOOL("zilsd", ext_zilsd, false), > MULTI_EXT_CFG_BOOL("zimop", ext_zimop, false), > MULTI_EXT_CFG_BOOL("zcmop", ext_zcmop, false), > MULTI_EXT_CFG_BOOL("zacas", ext_zacas, false), > diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc > index a154ecdc79..8d8e35e4cf 100644 > --- a/target/riscv/cpu_cfg_fields.h.inc > +++ b/target/riscv/cpu_cfg_fields.h.inc > @@ -41,6 +41,7 @@ BOOL_FIELD(ext_zicond) > BOOL_FIELD(ext_zihintntl) > BOOL_FIELD(ext_zihintpause) > BOOL_FIELD(ext_zihpm) > +BOOL_FIELD(ext_zilsd) > BOOL_FIELD(ext_zimop) > BOOL_FIELD(ext_zcmop) > BOOL_FIELD(ext_ztso) > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc > index 54b9b4f241..dcbb3811d9 100644 > --- a/target/riscv/insn_trans/trans_rvi.c.inc > +++ b/target/riscv/insn_trans/trans_rvi.c.inc > @@ -370,6 +370,27 @@ static bool gen_load_tl(DisasContext *ctx, arg_lb *a, MemOp memop) > return true; > } > > +/* Zilsd extension adds load/store double for 32bit arch */ > +static bool gen_load_zilsd(DisasContext *ctx, arg_lb *a) > +{ > + TCGv dest_1 = dest_gpr(ctx, a->rd); > + TCGv dest_2 = dest_gpr(ctx, (a->rd)+1); > + TCGv addr_1 = get_address(ctx, a->rs1, a->imm); > + TCGv addr_2 = get_address(ctx, a->rs1, (a->imm)+4); > + > + tcg_gen_qemu_ld_tl(dest_1, addr_1, ctx->mem_idx, MO_SL); > + tcg_gen_qemu_ld_tl(dest_2, addr_2, ctx->mem_idx, MO_SL); > + > + /* If destination is x0 then result of the load is discarded */ > + if (a->rd == 0) { > + return true; > + } Can't we exit earlier in this case? We're doing 2 dst_gpr() calls, 2 get_address() and 2 ld_tl() loads before checking a->rd == 0. Maybe something like this: > + TCGv addr_1, addr_2, dest_1, dest_2; > + > + /* If destination is x0 then result of the load is discarded */ > + if (a->rd == 0) { > + return true; > + } > + > + dest_1 = dest_gpr(ctx, a->rd); > + dest_2 = dest_gpr(ctx, (a->rd)+1); > + addr_1 = get_address(ctx, a->rs1, a->imm); > + addr_2 = get_address(ctx, a->rs1, (a->imm)+4); > + > + tcg_gen_qemu_ld_tl(dest_1, addr_1, ctx->mem_idx, MO_SL); > + tcg_gen_qemu_ld_tl(dest_2, addr_2, ctx->mem_idx, MO_SL); Thanks, Daniel > + > + gen_set_gpr(ctx, a->rd, dest_1); > + gen_set_gpr(ctx, a->rd+1, dest_2); > + return true; > +} > + > /* Compute only 64-bit addresses to use the address translation mechanism */ > static bool gen_load_i128(DisasContext *ctx, arg_lb *a, MemOp memop) > { > @@ -409,6 +430,8 @@ static bool gen_load(DisasContext *ctx, arg_lb *a, MemOp memop) > decode_save_opc(ctx, 0); > if (get_xl(ctx) == MXL_RV128) { > out = gen_load_i128(ctx, a, memop); > + } else if (memop == MO_SQ && get_xl(ctx) == MXL_RV32) { > + out = gen_load_zilsd(ctx, a); > } else { > out = gen_load_tl(ctx, a, memop); > } > @@ -437,7 +460,10 @@ static bool trans_lw(DisasContext *ctx, arg_lw *a) > > static bool trans_ld(DisasContext *ctx, arg_ld *a) > { > - REQUIRE_64_OR_128BIT(ctx); > + /* Check for Zilsd extension if 32bit */ > + if (get_xl(ctx) == MXL_RV32 && !ctx->cfg_ptr->ext_zilsd) { > + return false; > + } > return gen_load(ctx, a, MO_SQ); > } > > @@ -482,6 +508,27 @@ static bool gen_store_tl(DisasContext *ctx, arg_sb *a, MemOp memop) > return true; > } > > +/* Zilsd extension adds load/store double for 32bit arch */ > +static bool gen_store_zilsd(DisasContext *ctx, arg_sb *a) > +{ > + TCGv data_1 = tcg_temp_new(); > + TCGv data_2 = tcg_temp_new(); > + if (a->rs2 != 0) { > + data_1 = get_gpr(ctx, a->rs2, EXT_NONE); > + data_2 = get_gpr(ctx, (a->rs2)+1, EXT_NONE); > + } > + TCGv addr_1 = get_address(ctx, a->rs1, a->imm); > + TCGv addr_2 = get_address(ctx, a->rs1, (a->imm)+4); > + > + if (ctx->ztso) { > + tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); > + } > + > + tcg_gen_qemu_st_tl(data_1, addr_1, ctx->mem_idx, MO_SL); > + tcg_gen_qemu_st_tl(data_2, addr_2, ctx->mem_idx, MO_SL); > + return true; > +} > + > static bool gen_store_i128(DisasContext *ctx, arg_sb *a, MemOp memop) > { > TCGv src1l = get_gpr(ctx, a->rs1, EXT_NONE); > @@ -511,6 +558,8 @@ static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop) > decode_save_opc(ctx, 0); > if (get_xl(ctx) == MXL_RV128) { > return gen_store_i128(ctx, a, memop); > + } else if (memop == MO_UQ && get_xl(ctx) == MXL_RV32) { > + return gen_store_zilsd(ctx, a); > } else { > return gen_store_tl(ctx, a, memop); > } > @@ -533,7 +582,10 @@ static bool trans_sw(DisasContext *ctx, arg_sw *a) > > static bool trans_sd(DisasContext *ctx, arg_sd *a) > { > - REQUIRE_64_OR_128BIT(ctx); > + /* Check for Zilsd extension if 32bit */ > + if (get_xl(ctx) == MXL_RV32 && !ctx->cfg_ptr->ext_zilsd) { > + return false; > + } > return gen_store(ctx, a, MO_UQ); > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Add RISCV Zilsd extension 2025-11-06 12:17 ` Daniel Henrique Barboza @ 2025-11-06 14:31 ` Roan Richmond 2025-11-06 14:47 ` Daniel Henrique Barboza 0 siblings, 1 reply; 5+ messages in thread From: Roan Richmond @ 2025-11-06 14:31 UTC (permalink / raw) To: Daniel Henrique Barboza, qemu-riscv Cc: palmer, alistair.francis, liwei1518, zhiwei_liu, qemu-devel, alistair23 I understand your point about doing the check before the 2 ld_tl() and the 2 dst_gpr() calls. My reasoning for doing the check after was the wording of the specification: "LD instructions with destination x0 are processed as any other load, but the result is discarded entirely and x1 is not written" This suggests that a load instruction is still dispatched but the result is then discarded. I am happy to move the check if you think it is definitely necessary, otherwise I would prefer to leave it as happening after the calls to ld_tl() and dst_gpr, so we are closer the the expected behavior. Thanks, Roan On 06/11/2025 12:17, Daniel Henrique Barboza wrote: > > > On 11/4/25 8:59 AM, Roan Richmond wrote: >> This is based on v1.0 of the Zilsd extension [1]. >> The specification is listed as in the Ratified state [2], >> since Jan 30, 2025. >> >> [1]: https://github.com/riscv/riscv-zilsd >> [2]: >> https://riscv.atlassian.net/wiki/spaces/HOME/pages/16154861/RISC-V+Specs+Under+Development >> >> Signed-off-by: Roan Richmond <roan.richmond@codethink.co.uk> >> --- >> target/riscv/cpu.c | 2 + >> target/riscv/cpu_cfg_fields.h.inc | 1 + >> target/riscv/insn_trans/trans_rvi.c.inc | 56 ++++++++++++++++++++++++- >> 3 files changed, 57 insertions(+), 2 deletions(-) >> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >> index 73d4280d7c..6e0d37fb96 100644 >> --- a/target/riscv/cpu.c >> +++ b/target/riscv/cpu.c >> @@ -121,6 +121,7 @@ const RISCVIsaExtData isa_edata_arr[] = { >> ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl), >> ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, >> ext_zihintpause), >> ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm), >> + ISA_EXT_DATA_ENTRY(zilsd, PRIV_VERSION_1_13_0, ext_zilsd), >> ISA_EXT_DATA_ENTRY(zimop, PRIV_VERSION_1_13_0, ext_zimop), >> ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul), >> ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, has_priv_1_12), >> @@ -1247,6 +1248,7 @@ const RISCVCPUMultiExtConfig >> riscv_cpu_extensions[] = { >> MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true), >> MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true), >> MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true), >> + MULTI_EXT_CFG_BOOL("zilsd", ext_zilsd, false), >> MULTI_EXT_CFG_BOOL("zimop", ext_zimop, false), >> MULTI_EXT_CFG_BOOL("zcmop", ext_zcmop, false), >> MULTI_EXT_CFG_BOOL("zacas", ext_zacas, false), >> diff --git a/target/riscv/cpu_cfg_fields.h.inc >> b/target/riscv/cpu_cfg_fields.h.inc >> index a154ecdc79..8d8e35e4cf 100644 >> --- a/target/riscv/cpu_cfg_fields.h.inc >> +++ b/target/riscv/cpu_cfg_fields.h.inc >> @@ -41,6 +41,7 @@ BOOL_FIELD(ext_zicond) >> BOOL_FIELD(ext_zihintntl) >> BOOL_FIELD(ext_zihintpause) >> BOOL_FIELD(ext_zihpm) >> +BOOL_FIELD(ext_zilsd) >> BOOL_FIELD(ext_zimop) >> BOOL_FIELD(ext_zcmop) >> BOOL_FIELD(ext_ztso) >> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc >> b/target/riscv/insn_trans/trans_rvi.c.inc >> index 54b9b4f241..dcbb3811d9 100644 >> --- a/target/riscv/insn_trans/trans_rvi.c.inc >> +++ b/target/riscv/insn_trans/trans_rvi.c.inc >> @@ -370,6 +370,27 @@ static bool gen_load_tl(DisasContext *ctx, >> arg_lb *a, MemOp memop) >> return true; >> } >> +/* Zilsd extension adds load/store double for 32bit arch */ >> +static bool gen_load_zilsd(DisasContext *ctx, arg_lb *a) >> +{ >> + TCGv dest_1 = dest_gpr(ctx, a->rd); >> + TCGv dest_2 = dest_gpr(ctx, (a->rd)+1); >> + TCGv addr_1 = get_address(ctx, a->rs1, a->imm); >> + TCGv addr_2 = get_address(ctx, a->rs1, (a->imm)+4); >> + >> + tcg_gen_qemu_ld_tl(dest_1, addr_1, ctx->mem_idx, MO_SL); >> + tcg_gen_qemu_ld_tl(dest_2, addr_2, ctx->mem_idx, MO_SL); >> + >> + /* If destination is x0 then result of the load is discarded */ >> + if (a->rd == 0) { >> + return true; >> + } > > Can't we exit earlier in this case? We're doing 2 dst_gpr() calls, 2 > get_address() and > 2 ld_tl() loads before checking a->rd == 0. Maybe something like this: > > >> + TCGv addr_1, addr_2, dest_1, dest_2; >> + >> + /* If destination is x0 then result of the load is discarded */ >> + if (a->rd == 0) { >> + return true; >> + } >> + >> + dest_1 = dest_gpr(ctx, a->rd); >> + dest_2 = dest_gpr(ctx, (a->rd)+1); >> + addr_1 = get_address(ctx, a->rs1, a->imm); >> + addr_2 = get_address(ctx, a->rs1, (a->imm)+4); >> + >> + tcg_gen_qemu_ld_tl(dest_1, addr_1, ctx->mem_idx, MO_SL); >> + tcg_gen_qemu_ld_tl(dest_2, addr_2, ctx->mem_idx, MO_SL); > > > Thanks, > > Daniel > > > >> + >> + gen_set_gpr(ctx, a->rd, dest_1); >> + gen_set_gpr(ctx, a->rd+1, dest_2); >> + return true; >> +} >> + >> /* Compute only 64-bit addresses to use the address translation >> mechanism */ >> static bool gen_load_i128(DisasContext *ctx, arg_lb *a, MemOp memop) >> { >> @@ -409,6 +430,8 @@ static bool gen_load(DisasContext *ctx, arg_lb >> *a, MemOp memop) >> decode_save_opc(ctx, 0); >> if (get_xl(ctx) == MXL_RV128) { >> out = gen_load_i128(ctx, a, memop); >> + } else if (memop == MO_SQ && get_xl(ctx) == MXL_RV32) { >> + out = gen_load_zilsd(ctx, a); >> } else { >> out = gen_load_tl(ctx, a, memop); >> } >> @@ -437,7 +460,10 @@ static bool trans_lw(DisasContext *ctx, arg_lw *a) >> static bool trans_ld(DisasContext *ctx, arg_ld *a) >> { >> - REQUIRE_64_OR_128BIT(ctx); >> + /* Check for Zilsd extension if 32bit */ >> + if (get_xl(ctx) == MXL_RV32 && !ctx->cfg_ptr->ext_zilsd) { >> + return false; >> + } >> return gen_load(ctx, a, MO_SQ); >> } >> @@ -482,6 +508,27 @@ static bool gen_store_tl(DisasContext *ctx, >> arg_sb *a, MemOp memop) >> return true; >> } >> +/* Zilsd extension adds load/store double for 32bit arch */ >> +static bool gen_store_zilsd(DisasContext *ctx, arg_sb *a) >> +{ >> + TCGv data_1 = tcg_temp_new(); >> + TCGv data_2 = tcg_temp_new(); >> + if (a->rs2 != 0) { >> + data_1 = get_gpr(ctx, a->rs2, EXT_NONE); >> + data_2 = get_gpr(ctx, (a->rs2)+1, EXT_NONE); >> + } >> + TCGv addr_1 = get_address(ctx, a->rs1, a->imm); >> + TCGv addr_2 = get_address(ctx, a->rs1, (a->imm)+4); >> + >> + if (ctx->ztso) { >> + tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); >> + } >> + >> + tcg_gen_qemu_st_tl(data_1, addr_1, ctx->mem_idx, MO_SL); >> + tcg_gen_qemu_st_tl(data_2, addr_2, ctx->mem_idx, MO_SL); >> + return true; >> +} >> + >> static bool gen_store_i128(DisasContext *ctx, arg_sb *a, MemOp memop) >> { >> TCGv src1l = get_gpr(ctx, a->rs1, EXT_NONE); >> @@ -511,6 +558,8 @@ static bool gen_store(DisasContext *ctx, arg_sb >> *a, MemOp memop) >> decode_save_opc(ctx, 0); >> if (get_xl(ctx) == MXL_RV128) { >> return gen_store_i128(ctx, a, memop); >> + } else if (memop == MO_UQ && get_xl(ctx) == MXL_RV32) { >> + return gen_store_zilsd(ctx, a); >> } else { >> return gen_store_tl(ctx, a, memop); >> } >> @@ -533,7 +582,10 @@ static bool trans_sw(DisasContext *ctx, arg_sw *a) >> static bool trans_sd(DisasContext *ctx, arg_sd *a) >> { >> - REQUIRE_64_OR_128BIT(ctx); >> + /* Check for Zilsd extension if 32bit */ >> + if (get_xl(ctx) == MXL_RV32 && !ctx->cfg_ptr->ext_zilsd) { >> + return false; >> + } >> return gen_store(ctx, a, MO_UQ); >> } > > -- Roan Richmond (he/him) Software Engineer Codethink Ltd ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Add RISCV Zilsd extension 2025-11-06 14:31 ` Roan Richmond @ 2025-11-06 14:47 ` Daniel Henrique Barboza 2025-11-06 15:12 ` Richard Henderson 0 siblings, 1 reply; 5+ messages in thread From: Daniel Henrique Barboza @ 2025-11-06 14:47 UTC (permalink / raw) To: Roan Richmond, qemu-riscv Cc: palmer, alistair.francis, liwei1518, zhiwei_liu, qemu-devel, alistair23 On 11/6/25 11:31 AM, Roan Richmond wrote: > I understand your point about doing the check before the 2 ld_tl() and the 2 dst_gpr() calls. > > My reasoning for doing the check after was the wording of the specification: > "LD instructions with destination x0 are processed as any other load, but the result is discarded entirely and x1 is not written" > This suggests that a load instruction is still dispatched but the result is then discarded. Oh, so it's not a coding choice. In this case let's adhere to the spec as close as we can (a.k.a keep it as is). Thanks, Daniel > > I am happy to move the check if you think it is definitely necessary, otherwise I would prefer to leave it as happening after the calls to ld_tl() and dst_gpr, so we are closer the the expected behavior. > > Thanks, > Roan > > On 06/11/2025 12:17, Daniel Henrique Barboza wrote: >> >> >> On 11/4/25 8:59 AM, Roan Richmond wrote: >>> This is based on v1.0 of the Zilsd extension [1]. >>> The specification is listed as in the Ratified state [2], >>> since Jan 30, 2025. >>> >>> [1]: https://github.com/riscv/riscv-zilsd >>> [2]: https://riscv.atlassian.net/wiki/spaces/HOME/pages/16154861/RISC-V+Specs+Under+Development >>> >>> Signed-off-by: Roan Richmond <roan.richmond@codethink.co.uk> >>> --- >>> target/riscv/cpu.c | 2 + >>> target/riscv/cpu_cfg_fields.h.inc | 1 + >>> target/riscv/insn_trans/trans_rvi.c.inc | 56 ++++++++++++++++++++++++- >>> 3 files changed, 57 insertions(+), 2 deletions(-) >>> >>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >>> index 73d4280d7c..6e0d37fb96 100644 >>> --- a/target/riscv/cpu.c >>> +++ b/target/riscv/cpu.c >>> @@ -121,6 +121,7 @@ const RISCVIsaExtData isa_edata_arr[] = { >>> ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl), >>> ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), >>> ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm), >>> + ISA_EXT_DATA_ENTRY(zilsd, PRIV_VERSION_1_13_0, ext_zilsd), >>> ISA_EXT_DATA_ENTRY(zimop, PRIV_VERSION_1_13_0, ext_zimop), >>> ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul), >>> ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, has_priv_1_12), >>> @@ -1247,6 +1248,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = { >>> MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true), >>> MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true), >>> MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true), >>> + MULTI_EXT_CFG_BOOL("zilsd", ext_zilsd, false), >>> MULTI_EXT_CFG_BOOL("zimop", ext_zimop, false), >>> MULTI_EXT_CFG_BOOL("zcmop", ext_zcmop, false), >>> MULTI_EXT_CFG_BOOL("zacas", ext_zacas, false), >>> diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc >>> index a154ecdc79..8d8e35e4cf 100644 >>> --- a/target/riscv/cpu_cfg_fields.h.inc >>> +++ b/target/riscv/cpu_cfg_fields.h.inc >>> @@ -41,6 +41,7 @@ BOOL_FIELD(ext_zicond) >>> BOOL_FIELD(ext_zihintntl) >>> BOOL_FIELD(ext_zihintpause) >>> BOOL_FIELD(ext_zihpm) >>> +BOOL_FIELD(ext_zilsd) >>> BOOL_FIELD(ext_zimop) >>> BOOL_FIELD(ext_zcmop) >>> BOOL_FIELD(ext_ztso) >>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc >>> index 54b9b4f241..dcbb3811d9 100644 >>> --- a/target/riscv/insn_trans/trans_rvi.c.inc >>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc >>> @@ -370,6 +370,27 @@ static bool gen_load_tl(DisasContext *ctx, arg_lb *a, MemOp memop) >>> return true; >>> } >>> +/* Zilsd extension adds load/store double for 32bit arch */ >>> +static bool gen_load_zilsd(DisasContext *ctx, arg_lb *a) >>> +{ >>> + TCGv dest_1 = dest_gpr(ctx, a->rd); >>> + TCGv dest_2 = dest_gpr(ctx, (a->rd)+1); >>> + TCGv addr_1 = get_address(ctx, a->rs1, a->imm); >>> + TCGv addr_2 = get_address(ctx, a->rs1, (a->imm)+4); >>> + >>> + tcg_gen_qemu_ld_tl(dest_1, addr_1, ctx->mem_idx, MO_SL); >>> + tcg_gen_qemu_ld_tl(dest_2, addr_2, ctx->mem_idx, MO_SL); >>> + >>> + /* If destination is x0 then result of the load is discarded */ >>> + if (a->rd == 0) { >>> + return true; >>> + } >> >> Can't we exit earlier in this case? We're doing 2 dst_gpr() calls, 2 get_address() and >> 2 ld_tl() loads before checking a->rd == 0. Maybe something like this: >> >> >>> + TCGv addr_1, addr_2, dest_1, dest_2; >>> + >>> + /* If destination is x0 then result of the load is discarded */ >>> + if (a->rd == 0) { >>> + return true; >>> + } >>> + >>> + dest_1 = dest_gpr(ctx, a->rd); >>> + dest_2 = dest_gpr(ctx, (a->rd)+1); >>> + addr_1 = get_address(ctx, a->rs1, a->imm); >>> + addr_2 = get_address(ctx, a->rs1, (a->imm)+4); >>> + >>> + tcg_gen_qemu_ld_tl(dest_1, addr_1, ctx->mem_idx, MO_SL); >>> + tcg_gen_qemu_ld_tl(dest_2, addr_2, ctx->mem_idx, MO_SL); >> >> >> Thanks, >> >> Daniel >> >> >> >>> + >>> + gen_set_gpr(ctx, a->rd, dest_1); >>> + gen_set_gpr(ctx, a->rd+1, dest_2); >>> + return true; >>> +} >>> + >>> /* Compute only 64-bit addresses to use the address translation mechanism */ >>> static bool gen_load_i128(DisasContext *ctx, arg_lb *a, MemOp memop) >>> { >>> @@ -409,6 +430,8 @@ static bool gen_load(DisasContext *ctx, arg_lb *a, MemOp memop) >>> decode_save_opc(ctx, 0); >>> if (get_xl(ctx) == MXL_RV128) { >>> out = gen_load_i128(ctx, a, memop); >>> + } else if (memop == MO_SQ && get_xl(ctx) == MXL_RV32) { >>> + out = gen_load_zilsd(ctx, a); >>> } else { >>> out = gen_load_tl(ctx, a, memop); >>> } >>> @@ -437,7 +460,10 @@ static bool trans_lw(DisasContext *ctx, arg_lw *a) >>> static bool trans_ld(DisasContext *ctx, arg_ld *a) >>> { >>> - REQUIRE_64_OR_128BIT(ctx); >>> + /* Check for Zilsd extension if 32bit */ >>> + if (get_xl(ctx) == MXL_RV32 && !ctx->cfg_ptr->ext_zilsd) { >>> + return false; >>> + } >>> return gen_load(ctx, a, MO_SQ); >>> } >>> @@ -482,6 +508,27 @@ static bool gen_store_tl(DisasContext *ctx, arg_sb *a, MemOp memop) >>> return true; >>> } >>> +/* Zilsd extension adds load/store double for 32bit arch */ >>> +static bool gen_store_zilsd(DisasContext *ctx, arg_sb *a) >>> +{ >>> + TCGv data_1 = tcg_temp_new(); >>> + TCGv data_2 = tcg_temp_new(); >>> + if (a->rs2 != 0) { >>> + data_1 = get_gpr(ctx, a->rs2, EXT_NONE); >>> + data_2 = get_gpr(ctx, (a->rs2)+1, EXT_NONE); >>> + } >>> + TCGv addr_1 = get_address(ctx, a->rs1, a->imm); >>> + TCGv addr_2 = get_address(ctx, a->rs1, (a->imm)+4); >>> + >>> + if (ctx->ztso) { >>> + tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); >>> + } >>> + >>> + tcg_gen_qemu_st_tl(data_1, addr_1, ctx->mem_idx, MO_SL); >>> + tcg_gen_qemu_st_tl(data_2, addr_2, ctx->mem_idx, MO_SL); >>> + return true; >>> +} >>> + >>> static bool gen_store_i128(DisasContext *ctx, arg_sb *a, MemOp memop) >>> { >>> TCGv src1l = get_gpr(ctx, a->rs1, EXT_NONE); >>> @@ -511,6 +558,8 @@ static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop) >>> decode_save_opc(ctx, 0); >>> if (get_xl(ctx) == MXL_RV128) { >>> return gen_store_i128(ctx, a, memop); >>> + } else if (memop == MO_UQ && get_xl(ctx) == MXL_RV32) { >>> + return gen_store_zilsd(ctx, a); >>> } else { >>> return gen_store_tl(ctx, a, memop); >>> } >>> @@ -533,7 +582,10 @@ static bool trans_sw(DisasContext *ctx, arg_sw *a) >>> static bool trans_sd(DisasContext *ctx, arg_sd *a) >>> { >>> - REQUIRE_64_OR_128BIT(ctx); >>> + /* Check for Zilsd extension if 32bit */ >>> + if (get_xl(ctx) == MXL_RV32 && !ctx->cfg_ptr->ext_zilsd) { >>> + return false; >>> + } >>> return gen_store(ctx, a, MO_UQ); >>> } >> >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Add RISCV Zilsd extension 2025-11-06 14:47 ` Daniel Henrique Barboza @ 2025-11-06 15:12 ` Richard Henderson 0 siblings, 0 replies; 5+ messages in thread From: Richard Henderson @ 2025-11-06 15:12 UTC (permalink / raw) To: Daniel Henrique Barboza, Roan Richmond, qemu-riscv Cc: palmer, alistair.francis, liwei1518, zhiwei_liu, qemu-devel, alistair23 On 11/6/25 15:47, Daniel Henrique Barboza wrote: > > > On 11/6/25 11:31 AM, Roan Richmond wrote: >> I understand your point about doing the check before the 2 ld_tl() and the 2 dst_gpr() >> calls. >> >> My reasoning for doing the check after was the wording of the specification: >> "LD instructions with destination x0 are processed as any other load, but the result is >> discarded entirely and x1 is not written" >> This suggests that a load instruction is still dispatched but the result is then discarded. ... >>>> +/* Zilsd extension adds load/store double for 32bit arch */ >>>> +static bool gen_load_zilsd(DisasContext *ctx, arg_lb *a) >>>> +{ >>>> + TCGv dest_1 = dest_gpr(ctx, a->rd); >>>> + TCGv dest_2 = dest_gpr(ctx, (a->rd)+1); >>>> + TCGv addr_1 = get_address(ctx, a->rs1, a->imm); >>>> + TCGv addr_2 = get_address(ctx, a->rs1, (a->imm)+4); >>>> + >>>> + tcg_gen_qemu_ld_tl(dest_1, addr_1, ctx->mem_idx, MO_SL); >>>> + tcg_gen_qemu_ld_tl(dest_2, addr_2, ctx->mem_idx, MO_SL); >>>> + >>>> + /* If destination is x0 then result of the load is discarded */ >>>> + if (a->rd == 0) { >>>> + return true; >>>> + } If you're looking to not write to r1, then you need to use true temporaries, not dest_gpr(), which may return r1, which will be modified by the load. You can drop the unnecessary () in those a->{rd,imm} + c expressions. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-11-06 15:12 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-04 11:59 [PATCH] Add RISCV Zilsd extension Roan Richmond 2025-11-06 12:17 ` Daniel Henrique Barboza 2025-11-06 14:31 ` Roan Richmond 2025-11-06 14:47 ` Daniel Henrique Barboza 2025-11-06 15:12 ` Richard Henderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).