| Message ID | 20201014170159.26932-6-space.monkey.delivers@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/5,RISCV_PM] Add J-extension into RISC-V | expand |
On 10/14/20 10:01 AM, Alexey Baturo wrote: > + if (has_ext(ctx, RVJ)) { > + src1 = apply_pointer_masking(ctx, src1); > + } The if is redundant, since that will have been done in cpu_get_tb_cpu_state while assigning pm_enabled. The test for pm_enabled is in gen_pm_adjust_address. The final thing is that the API for apply_pointer_masking is misleading. Here, it appears as if you are allocating a new temporary and assigning it to src1. Which is not the case. I suggest you drop apply_pointer_masking and just use gen_pm_adjust_address. r~
> The if is redundant, since that will have been done in cpu_get_tb_cpu_state while assigning pm_enabled. I totally agree here, however I tried to do explicit checks, so this functionality is available only if a special option is supplied. But if you think that's too much and I could do just mov(dst, src) in case PM is disabled, I'd gladly fix that. > I suggest you drop apply_pointer_masking and just use gen_pm_adjust_address. Sure, will do. Thanks! ср, 14 окт. 2020 г. в 22:24, Richard Henderson <richard.henderson@linaro.org >: > On 10/14/20 10:01 AM, Alexey Baturo wrote: > > + if (has_ext(ctx, RVJ)) { > > + src1 = apply_pointer_masking(ctx, src1); > > + } > > The if is redundant, since that will have been done in cpu_get_tb_cpu_state > while assigning pm_enabled. > > The test for pm_enabled is in gen_pm_adjust_address. > > The final thing is that the API for apply_pointer_masking is misleading. > Here, > it appears as if you are allocating a new temporary and assigning it to > src1. > Which is not the case. > > I suggest you drop apply_pointer_masking and just use > gen_pm_adjust_address. > > > r~ > <div dir="ltr">> The if is redundant, since that will have been done in cpu_get_tb_cpu_state while assigning pm_enabled.<div>I totally agree here, however I tried to do explicit checks, so this functionality is available only if a special option is supplied.</div><div>But if you think that's too much and I could do just mov(dst, src) in case PM is disabled, I'd gladly fix that.</div><div><br></div><div>> I suggest you drop apply_pointer_masking and just use gen_pm_adjust_address.</div><div>Sure, will do.</div><div><br></div><div>Thanks!</div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">ср, 14 окт. 2020 г. в 22:24, Richard Henderson <<a href="mailto:richard.henderson@linaro.org">richard.henderson@linaro.org</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 10/14/20 10:01 AM, Alexey Baturo wrote:<br> > + if (has_ext(ctx, RVJ)) {<br> > + src1 = apply_pointer_masking(ctx, src1);<br> > + }<br> <br> The if is redundant, since that will have been done in cpu_get_tb_cpu_state<br> while assigning pm_enabled.<br> <br> The test for pm_enabled is in gen_pm_adjust_address.<br> <br> The final thing is that the API for apply_pointer_masking is misleading. Here,<br> it appears as if you are allocating a new temporary and assigning it to src1.<br> Which is not the case.<br> <br> I suggest you drop apply_pointer_masking and just use gen_pm_adjust_address.<br> <br> <br> r~<br> </blockquote></div>
diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc index be8a9f06dd..3bf2e82013 100644 --- a/target/riscv/insn_trans/trans_rva.c.inc +++ b/target/riscv/insn_trans/trans_rva.c.inc @@ -26,6 +26,9 @@ static inline bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop) if (a->rl) { tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); } + if (has_ext(ctx, RVJ)) { + src1 = apply_pointer_masking(ctx, src1); + } tcg_gen_qemu_ld_tl(load_val, src1, ctx->mem_idx, mop); if (a->aq) { tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ); @@ -46,6 +49,9 @@ static inline bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop) TCGLabel *l2 = gen_new_label(); gen_get_gpr(src1, a->rs1); + if (has_ext(ctx, RVJ)) { + src1 = apply_pointer_masking(ctx, src1); + } tcg_gen_brcond_tl(TCG_COND_NE, load_res, src1, l1); gen_get_gpr(src2, a->rs2); @@ -91,6 +97,9 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a, gen_get_gpr(src1, a->rs1); gen_get_gpr(src2, a->rs2); + if (has_ext(ctx, RVJ)) { + src1 = apply_pointer_masking(ctx, src1); + } (*func)(src2, src1, src2, ctx->mem_idx, mop); gen_set_gpr(a->rd, src2); diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc index 4f832637fa..0391bb02be 100644 --- a/target/riscv/insn_trans/trans_rvd.c.inc +++ b/target/riscv/insn_trans/trans_rvd.c.inc @@ -25,6 +25,9 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a) TCGv t0 = tcg_temp_new(); gen_get_gpr(t0, a->rs1); tcg_gen_addi_tl(t0, t0, a->imm); + if (has_ext(ctx, RVJ)) { + t0 = apply_pointer_masking(ctx, t0); + } tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], t0, ctx->mem_idx, MO_TEQ); @@ -40,6 +43,9 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a) TCGv t0 = tcg_temp_new(); gen_get_gpr(t0, a->rs1); tcg_gen_addi_tl(t0, t0, a->imm); + if (has_ext(ctx, RVJ)) { + t0 = apply_pointer_masking(ctx, t0); + } tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], t0, ctx->mem_idx, MO_TEQ); diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc index 3dfec8211d..176bc992e1 100644 --- a/target/riscv/insn_trans/trans_rvf.c.inc +++ b/target/riscv/insn_trans/trans_rvf.c.inc @@ -30,6 +30,9 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a) TCGv t0 = tcg_temp_new(); gen_get_gpr(t0, a->rs1); tcg_gen_addi_tl(t0, t0, a->imm); + if (has_ext(ctx, RVJ)) { + t0 = apply_pointer_masking(ctx, t0); + } tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], t0, ctx->mem_idx, MO_TEUL); gen_nanbox_s(cpu_fpr[a->rd], cpu_fpr[a->rd]); @@ -47,6 +50,9 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a) gen_get_gpr(t0, a->rs1); tcg_gen_addi_tl(t0, t0, a->imm); + if (has_ext(ctx, RVJ)) { + t0 = apply_pointer_masking(ctx, t0); + } tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], t0, ctx->mem_idx, MO_TEUL); diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc index d04ca0394c..3ee2fea271 100644 --- a/target/riscv/insn_trans/trans_rvi.c.inc +++ b/target/riscv/insn_trans/trans_rvi.c.inc @@ -141,6 +141,9 @@ static bool gen_load(DisasContext *ctx, arg_lb *a, MemOp memop) TCGv t1 = tcg_temp_new(); gen_get_gpr(t0, a->rs1); tcg_gen_addi_tl(t0, t0, a->imm); + if (has_ext(ctx, RVJ)) { + t0 = apply_pointer_masking(ctx, t0); + } tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, memop); gen_set_gpr(a->rd, t1); @@ -180,6 +183,9 @@ static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop) TCGv dat = tcg_temp_new(); gen_get_gpr(t0, a->rs1); tcg_gen_addi_tl(t0, t0, a->imm); + if (has_ext(ctx, RVJ)) { + t0 = apply_pointer_masking(ctx, t0); + } gen_get_gpr(dat, a->rs2); tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx, memop); diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 338a967e0c..0b086753d4 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -416,6 +416,9 @@ static void gen_load_c(DisasContext *ctx, uint32_t opc, int rd, int rs1, TCGv t1 = tcg_temp_new(); gen_get_gpr(t0, rs1); tcg_gen_addi_tl(t0, t0, imm); + if (has_ext(ctx, RVJ)) { + t0 = apply_pointer_masking(ctx, t0); + } int memop = tcg_memop_lookup[(opc >> 12) & 0x7]; if (memop < 0) { @@ -436,6 +439,9 @@ static void gen_store_c(DisasContext *ctx, uint32_t opc, int rs1, int rs2, TCGv dat = tcg_temp_new(); gen_get_gpr(t0, rs1); tcg_gen_addi_tl(t0, t0, imm); + if (has_ext(ctx, RVJ)) { + t0 = apply_pointer_masking(ctx, t0); + } gen_get_gpr(dat, rs2); int memop = tcg_memop_lookup[(opc >> 12) & 0x7]; @@ -495,6 +501,9 @@ static void gen_fp_load(DisasContext *ctx, uint32_t opc, int rd, t0 = tcg_temp_new(); gen_get_gpr(t0, rs1); tcg_gen_addi_tl(t0, t0, imm); + if (riscv_has_ext(env, RVJ)) { + t0 = apply_pointer_masking(ctx, t0); + } switch (opc) { case OPC_RISC_FLW: @@ -534,6 +543,9 @@ static void gen_fp_store(DisasContext *ctx, uint32_t opc, int rs1, t0 = tcg_temp_new(); gen_get_gpr(t0, rs1); tcg_gen_addi_tl(t0, t0, imm); + if (riscv_has_ext(env, RVJ)) { + t0 = apply_pointer_masking(ctx, t0); + } switch (opc) { case OPC_RISC_FSW:
Signed-off-by: Alexey Baturo <space.monkey.delivers@gmail.com> --- target/riscv/insn_trans/trans_rva.c.inc | 9 +++++++++ target/riscv/insn_trans/trans_rvd.c.inc | 6 ++++++ target/riscv/insn_trans/trans_rvf.c.inc | 6 ++++++ target/riscv/insn_trans/trans_rvi.c.inc | 6 ++++++ target/riscv/translate.c | 12 ++++++++++++ 5 files changed, 39 insertions(+)