Re: [PATCH v2] Add RISCV Zilsd extension

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

On 11/10/25 10:05, Roan Richmond wrote:
> +/* 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);

This is wrong, because if rs2 == 0, then data_1 and data_2 are uninitialized.  If you're 
testing properly with --enable-debug-tcg, this should trigger an assertion failure.

You wanted

     if (a->rs2 == 0) {
         data_1 = tcg_constant_tl(0);
         data_2 = data_1;
     } else {
         data_1 = get_gpr(ctx, a->rs2, EXT_NONE);
         data_2 = get_gpr(ctx, a->rs2 + 1, EXT_NONE);
     }

You're also missing the endianness indicator: mo_endian().

You really should consider combining the two halves so that you can perform one 64-bit 
store.  While I see that the spec allows the store to be non-atomic, you still probably do 
not want to to allow the first half store to succeed when the second half store faults.  I 
don't see clear language about that.  Anyway, that's as simple as

     TCGv_i64 data;
     MemOp end = mo_endian(ctx);

     if (a->rs2 == 0) {
         data = tcg_constant_i64(0);
     } else {
         TCGv data_1 = get_gpr(ctx, a->rs2, EXT_NONE);
         TCGv data_2 = get_gpr(ctx, a->rs2 + 1, EXT_NONE);
         data = tcg_temp_new_i64();
         if (end == MO_LE) {
             tcg_gen_concat_tl_i64(data, data_1, data_2);
         } else {
             tcg_gen_concat_tl_i64(data, data_2, data_1);
         }
     }
     tcg_gen_qemu_st_i64(data, addr, ctx->mem_idx, MO_UQ | end);


r~