diff mbox series

[5/5,RISCV_PM] Support pointer masking for RISC-V for i/c/f/d/a types of instructions

Message ID 20201014170159.26932-6-space.monkey.delivers@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/5,RISCV_PM] Add J-extension into RISC-V | expand

Commit Message

Alexey Baturo Oct. 14, 2020, 5:01 p.m. UTC 
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(+)

Comments

Richard Henderson Oct. 14, 2020, 7:24 p.m. UTC | #1 
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~
Alexey Baturo Oct. 14, 2020, 8:13 p.m. UTC | #2 
> 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~
>
diff mbox series

Patch

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: