[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 09/14] RISC-V: Adding T-Head MemIdx extension
From: |
Christoph Müllner |
Subject: |
Re: [PATCH v3 09/14] RISC-V: Adding T-Head MemIdx extension |
Date: |
Tue, 31 Jan 2023 19:01:51 +0100 |
On Tue, Jan 24, 2023 at 10:21 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 1/24/23 09:59, Christoph Muellner wrote:
> > +/* XTheadMemIdx */
> > +
> > +/*
> > + * Load with memop from indexed address and add (imm5 << imm2) to rs1.
> > + * If !preinc, then the load address is rs1.
> > + * If preinc, then the load address is rs1 + (imm5) << imm2).
> > + */
> > +static bool gen_load_inc(DisasContext *ctx, arg_th_meminc *a, MemOp memop,
> > + bool preinc)
> > +{
> > + TCGv rd = dest_gpr(ctx, a->rd);
> > + TCGv addr = get_address(ctx, a->rs1, preinc ? a->imm5 << a->imm2 : 0);
> > +
> > + tcg_gen_qemu_ld_tl(rd, addr, ctx->mem_idx, memop);
> > + addr = get_address(ctx, a->rs1, !preinc ? a->imm5 << a->imm2 : 0);
>
> First, you're leaking the previous 'addr' temporary.
Indeed!
The real question is of course, why we call get_address() twice...
Fixed in v4.
> Second, get_address may make modifications to 'addr' which you don't want to
> write back.
Fixed in v4.
> Third, you are not checking for rd != rs1.
Fixed in v4.
>
> I think what you want is
>
> int imm = a->imm5 << a->imm2;
> TCGv addr = get_address(ctx, a->rs1, preinc ? imm : 0);
> TCGv rd = dest_gpr(ctx, a->rd);
> TCGv rs1 = get_gpr(ctx, a->rs1, EXT_NONE);
>
> tcg_gen_qemu_ld_tl(rd, addr, ctx->mem_idx, memop);
> tcg_gen_addi_tl(rs1, rs1, imm);
> gen_set_gpr(ctx, a->rd, rd);
> gen_set_gpr(ctx, a->rs1, rs1);
Yes (plus the check for rd != rs1).
Fixed in v4.
Thank you!
>
>
> r~
- Re: [PATCH v3 08/14] RISC-V: Adding T-Head MemPair extension, (continued)
- Re: [PATCH v3 08/14] RISC-V: Adding T-Head MemPair extension, LIU Zhiwei, 2023/01/29
- Re: [PATCH v3 08/14] RISC-V: Adding T-Head MemPair extension, Richard Henderson, 2023/01/30
- Re: [PATCH v3 08/14] RISC-V: Adding T-Head MemPair extension, LIU Zhiwei, 2023/01/30
- Re: [PATCH v3 08/14] RISC-V: Adding T-Head MemPair extension, Richard Henderson, 2023/01/30
- Re: [PATCH v3 08/14] RISC-V: Adding T-Head MemPair extension, LIU Zhiwei, 2023/01/30
- Re: [PATCH v3 08/14] RISC-V: Adding T-Head MemPair extension, Christoph Müllner, 2023/01/31
[PATCH v3 10/14] RISC-V: Adding T-Head FMemIdx extension, Christoph Muellner, 2023/01/24
[PATCH v3 09/14] RISC-V: Adding T-Head MemIdx extension, Christoph Muellner, 2023/01/24
[PATCH v3 13/14] RISC-V: Adding XTheadFmv ISA extension, Christoph Muellner, 2023/01/24
[PATCH v3 11/14] RISC-V: Set minimum priv version for Zfh to 1.11, Christoph Muellner, 2023/01/24
[PATCH v3 12/14] RISC-V: Add initial support for T-Head C906, Christoph Muellner, 2023/01/24
[PATCH v3 14/14] target/riscv: add a MAINTAINERS entry for XThead* extension support, Christoph Muellner, 2023/01/24