| Message ID | 20181020071451.27808-7-kbastian@mail.uni-paderborn.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: Convert to decodetree | expand |
On 10/20/18 8:14 AM, Bastian Koppelmann wrote: > Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> > Signed-off-by: Peer Adelt <peer.adelt@hni.uni-paderborn.de> > --- > v1 -> v2: > - simplified fence/fence_i as suggested by Richard > > target/riscv/insn32.decode | 6 ++++++ > target/riscv/insn_trans/trans_rvi.inc.c | 20 ++++++++++++++++++++ > target/riscv/translate.c | 14 -------------- > 3 files changed, 26 insertions(+), 14 deletions(-) > > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode > index cb7622e223..00e30dbc71 100644 > --- a/target/riscv/insn32.decode > +++ b/target/riscv/insn32.decode > @@ -24,6 +24,9 @@ > %sh6 20:6 > %sh5 20:5 > > +%pred 24:4 > +%succ 20:4 Unused. > +static bool trans_fence(DisasContext *ctx, arg_fence *a, uint32_t insn) > +{ > +#ifndef CONFIG_USER_ONLY > + /* FENCE is a full memory barrier. */ > + tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); > +#endif > + return true; > +} > +static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a, uint32_t insn) Spacing. > +{ > +#ifndef CONFIG_USER_ONLY > + /* FENCE_I is a no-op in QEMU, > + * however we need to end the translation block */ > + tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn); > + tcg_gen_exit_tb(NULL, 0); > + ctx->base.is_jmp = DISAS_NORETURN; > +#endif > + return true; > +} I will note that both of these should apply to user-only as well... > - case OPC_RISC_FENCE: > -#ifndef CONFIG_USER_ONLY > - if (ctx->opcode & 0x1000) { > - /* FENCE_I is a no-op in QEMU, > - * however we need to end the translation block */ > - tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn); > - tcg_gen_exit_tb(NULL, 0); > - ctx->base.is_jmp = DISAS_NORETURN; > - } else { > - /* FENCE is a full memory barrier. */ > - tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); > - } > -#endif ... even though you're simply copying the bug. I mentioned this twice during initial review of risc-v, but it seems to have been missed during commit. Also note that one can do better than TCG_MO_ALL by actually using the pred/succ bits to select TCG_MO_{LD,ST}_{LD,ST}. That said, no new bugs with this patch, so Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode index cb7622e223..00e30dbc71 100644 --- a/target/riscv/insn32.decode +++ b/target/riscv/insn32.decode @@ -24,6 +24,9 @@ %sh6 20:6 %sh5 20:5 +%pred 24:4 +%succ 20:4 + # immediates: %imm_i 20:s12 %imm_s 25:s7 7:5 @@ -36,6 +39,7 @@ &shift shamt rs1 rd # Formats 32: + @r ....... ..... ..... ... ..... ....... %rs2 %rs1 %rd @i ............ ..... ... ..... ....... imm=%imm_i %rs1 %rd @b ....... ..... ..... ... ..... ....... &branch imm=%imm_b %rs2 %rs1 @@ -84,6 +88,8 @@ srl 0000000 ..... ..... 101 ..... 0110011 @r sra 0100000 ..... ..... 101 ..... 0110011 @r or 0000000 ..... ..... 110 ..... 0110011 @r and 0000000 ..... ..... 111 ..... 0110011 @r +fence ---- pred:4 succ:4 ----- 000 ----- 0001111 +fence_i ---- ---- ---- ----- 001 ----- 0001111 # *** RV64I Base Instruction Set (in addition to RV32I) *** lwu ............ ..... 110 ..... 0000011 @i diff --git a/target/riscv/insn_trans/trans_rvi.inc.c b/target/riscv/insn_trans/trans_rvi.inc.c index e5a67e64cb..14164a952d 100644 --- a/target/riscv/insn_trans/trans_rvi.inc.c +++ b/target/riscv/insn_trans/trans_rvi.inc.c @@ -341,3 +341,23 @@ static bool trans_sraw(DisasContext *ctx, arg_sraw *a, uint32_t insn) gen_arith(ctx, OPC_RISC_SRAW, a->rd, a->rs1, a->rs2); return true; } + +static bool trans_fence(DisasContext *ctx, arg_fence *a, uint32_t insn) +{ +#ifndef CONFIG_USER_ONLY + /* FENCE is a full memory barrier. */ + tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); +#endif + return true; +} +static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a, uint32_t insn) +{ +#ifndef CONFIG_USER_ONLY + /* FENCE_I is a no-op in QEMU, + * however we need to end the translation block */ + tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn); + tcg_gen_exit_tb(NULL, 0); + ctx->base.is_jmp = DISAS_NORETURN; +#endif + return true; +} diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 3e296a2627..f2567117b9 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -1740,20 +1740,6 @@ static void decode_RV32_64G(CPURISCVState *env, DisasContext *ctx) gen_fp_arith(ctx, MASK_OP_FP_ARITH(ctx->opcode), rd, rs1, rs2, GET_RM(ctx->opcode)); break; - case OPC_RISC_FENCE: -#ifndef CONFIG_USER_ONLY - if (ctx->opcode & 0x1000) { - /* FENCE_I is a no-op in QEMU, - * however we need to end the translation block */ - tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn); - tcg_gen_exit_tb(NULL, 0); - ctx->base.is_jmp = DISAS_NORETURN; - } else { - /* FENCE is a full memory barrier. */ - tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); - } -#endif - break; case OPC_RISC_SYSTEM: gen_system(env, ctx, MASK_OP_SYSTEM(ctx->opcode), rd, rs1, (ctx->opcode & 0xFFF00000) >> 20);