Re: [PATCH v5 09/18] target/riscv: accessors to registers upper part and 128-bit load/store

[Richard Henderson]

On 11/12/21 3:58 PM, Frédéric Pétrot wrote:
> +/* Compute only 64-bit addresses to use the address translation mechanism */
> +static bool gen_load_i128(DisasContext *ctx, arg_lb *a, MemOp memop)
> +{
> +    TCGv src1l = get_gpr(ctx, a->rs1, EXT_NONE);
> +    TCGv destl = dest_gpr(ctx, a->rd);
> +    TCGv desth = dest_gprh(ctx, a->rd);
> +    TCGv addrl = tcg_temp_new();
> +
> +    tcg_gen_addi_tl(addrl, src1l, a->imm);
> +
> +    if (memop != MO_TEUO) {

It is perhaps clearer as (memop & MO_SIZE) <= MO_64.

> +        tcg_gen_qemu_ld_tl(memop & MO_BSWAP ? desth : destl, addrl,
> +                           ctx->mem_idx, MO_TEUQ);

This isn't correct.  MO_BSWAP is related to the host, not the guest.

You want

        (memop & MO_BSWAP) == MO_LE ? destl : desth

Are there any big-endian RISC-V though?


> +static bool gen_store_i128(DisasContext *ctx, arg_sb *a, MemOp memop)
> +{
> +    TCGv src1l = get_gpr(ctx, a->rs1, EXT_NONE);
> +    TCGv src2l = get_gpr(ctx, a->rs2, EXT_NONE);
> +    TCGv src2h = get_gprh(ctx, a->rs2);
> +    TCGv addrl = tcg_temp_new();
> +
> +    tcg_gen_addi_tl(addrl, src1l, a->imm);
> +
> +    if (memop != MO_TEUO) {
> +        tcg_gen_qemu_st_tl(src2l, addrl, ctx->mem_idx, memop);
> +    } else {
> +        tcg_gen_qemu_st_tl(memop & MO_BSWAP ? src2h : src2l, addrl,
> +            ctx->mem_idx, MO_TEUQ);

Likewise.


r~