[PATCH 01/13] target/riscv: Really use little endianness for 128-bit loads/stores

Philippe Mathieu-Daudé posted 13 patches 1 month ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Christoph Muellner <christoph.muellner@vrull.eu>
[PATCH 01/13] target/riscv: Really use little endianness for 128-bit loads/stores
Posted by Philippe Mathieu-Daudé 1 month ago
Per commit a2f827ff4f4 ("target/riscv: accessors to registers upper
part and 128-bit load/store") description:

 > The 128-bit ISA adds ldu, lq and sq. We provide support for these
 > instructions. Note that (a) we compute only 64-bit addresses to
 > actually access memory, cowardly utilizing the existing address
 > translation mechanism of QEMU, and (b) we assume for now
 > little-endian memory accesses.

   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

However this commit used MO_TE (target endianness) for the
gen_load_i128() and gen_store_i128() helpers. Likely it was
unnoticed because current targets are only built using little
endianness:

  $ git grep -L TARGET_BIG_ENDIAN=y configs/targets/riscv*
  configs/targets/riscv32-linux-user.mak
  configs/targets/riscv32-softmmu.mak
  configs/targets/riscv64-bsd-user.mak
  configs/targets/riscv64-linux-user.mak
  configs/targets/riscv64-softmmu.mak

Replace by MO_TE -> MO_LE to really use little endianness.

Cc: Fabien Portas <fabien.portas@grenoble-inp.org>
Cc: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
Fixes: a2f827ff4f4 ("target/riscv: accessors to registers upper part and 128-bit load/store")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/riscv/insn_trans/trans_rvi.c.inc | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index b9c71604687..df0b555176a 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -389,9 +389,11 @@ static bool gen_load_i128(DisasContext *ctx, arg_lb *a, MemOp memop)
         }
     } else {
         /* assume little-endian memory access for now */
-        tcg_gen_qemu_ld_tl(destl, addrl, ctx->mem_idx, MO_TEUQ);
+        MemOp memop = MO_LEUQ;
+
+        tcg_gen_qemu_ld_tl(destl, addrl, ctx->mem_idx, memop);
         tcg_gen_addi_tl(addrl, addrl, 8);
-        tcg_gen_qemu_ld_tl(desth, addrl, ctx->mem_idx, MO_TEUQ);
+        tcg_gen_qemu_ld_tl(desth, addrl, ctx->mem_idx, memop);
     }
 
     gen_set_gpr128(ctx, a->rd, destl, desth);
@@ -494,9 +496,11 @@ static bool gen_store_i128(DisasContext *ctx, arg_sb *a, MemOp memop)
         tcg_gen_qemu_st_tl(src2l, addrl, ctx->mem_idx, memop);
     } else {
         /* little-endian memory access assumed for now */
-        tcg_gen_qemu_st_tl(src2l, addrl, ctx->mem_idx, MO_TEUQ);
+        MemOp memop = MO_LEUQ;
+
+        tcg_gen_qemu_st_tl(src2l, addrl, ctx->mem_idx, memop);
         tcg_gen_addi_tl(addrl, addrl, 8);
-        tcg_gen_qemu_st_tl(src2h, addrl, ctx->mem_idx, MO_TEUQ);
+        tcg_gen_qemu_st_tl(src2h, addrl, ctx->mem_idx, memop);
     }
     return true;
 }
-- 
2.51.0


Re: [PATCH 01/13] target/riscv: Really use little endianness for 128-bit loads/stores
Posted by Richard Henderson 1 month ago
On 10/10/25 08:50, Philippe Mathieu-Daudé wrote:
> Per commit a2f827ff4f4 ("target/riscv: accessors to registers upper
> part and 128-bit load/store") description:
> 
>   > The 128-bit ISA adds ldu, lq and sq. We provide support for these
>   > instructions. Note that (a) we compute only 64-bit addresses to
>   > actually access memory, cowardly utilizing the existing address
>   > translation mechanism of QEMU, and (b) we assume for now
>   > little-endian memory accesses.
> 
>     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> However this commit used MO_TE (target endianness) for the
> gen_load_i128() and gen_store_i128() helpers. Likely it was
> unnoticed because current targets are only built using little
> endianness:
> 
>    $ git grep -L TARGET_BIG_ENDIAN=y configs/targets/riscv*
>    configs/targets/riscv32-linux-user.mak
>    configs/targets/riscv32-softmmu.mak
>    configs/targets/riscv64-bsd-user.mak
>    configs/targets/riscv64-linux-user.mak
>    configs/targets/riscv64-softmmu.mak
> 
> Replace by MO_TE -> MO_LE to really use little endianness.
> 
> Cc: Fabien Portas <fabien.portas@grenoble-inp.org>
> Cc: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
> Fixes: a2f827ff4f4 ("target/riscv: accessors to registers upper part and 128-bit load/store")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/riscv/insn_trans/trans_rvi.c.inc | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
> index b9c71604687..df0b555176a 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -389,9 +389,11 @@ static bool gen_load_i128(DisasContext *ctx, arg_lb *a, MemOp memop)
>           }
>       } else {
>           /* assume little-endian memory access for now */
> -        tcg_gen_qemu_ld_tl(destl, addrl, ctx->mem_idx, MO_TEUQ);
> +        MemOp memop = MO_LEUQ;
> +
> +        tcg_gen_qemu_ld_tl(destl, addrl, ctx->mem_idx, memop);
>           tcg_gen_addi_tl(addrl, addrl, 8);
> -        tcg_gen_qemu_ld_tl(desth, addrl, ctx->mem_idx, MO_TEUQ);
> +        tcg_gen_qemu_ld_tl(desth, addrl, ctx->mem_idx, memop);
>       }
>   
>       gen_set_gpr128(ctx, a->rd, destl, desth);
> @@ -494,9 +496,11 @@ static bool gen_store_i128(DisasContext *ctx, arg_sb *a, MemOp memop)
>           tcg_gen_qemu_st_tl(src2l, addrl, ctx->mem_idx, memop);
>       } else {
>           /* little-endian memory access assumed for now */
> -        tcg_gen_qemu_st_tl(src2l, addrl, ctx->mem_idx, MO_TEUQ);
> +        MemOp memop = MO_LEUQ;
> +
> +        tcg_gen_qemu_st_tl(src2l, addrl, ctx->mem_idx, memop);
>           tcg_gen_addi_tl(addrl, addrl, 8);
> -        tcg_gen_qemu_st_tl(src2h, addrl, ctx->mem_idx, MO_TEUQ);
> +        tcg_gen_qemu_st_tl(src2h, addrl, ctx->mem_idx, memop);
>       }
>       return true;
>   }

We fix this to use tcg_gen_qemu_{ld,st}_i128.


r~