On 2023/1/25 5:21, Richard Henderson 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.
Although all temps allocated by temp_new() will be freed after the
instruction translation automatically, we can improve current
implementation.
Second, get_address may make modifications to 'addr' which you don't
want to write back.
Good catch.
Third, you are not checking for rd != rs1.
Yes.
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, we should write back the 'addr' without modification.
Best Regards,
Zhiwei
r~
