* [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).