On Wed, Jan 18, 2023 at 8:02 PM Daniel Henrique Barboza <dbarb...@ventanamicro.com> wrote: > > s/arm/riscv in subject/commit title ^
Fixed when committing Thanks! Applied to riscv-to-apply.next Alistair > > On 1/15/23 13:06, Richard Henderson wrote: > > The new helper always validates the contents of FRM, even > if the new rounding mode is not DYN. This is required by > the vector unit. > > Track whether we've validated FRM separately from whether > we've updated fp_status with a given rounding mode, so that > we can elide calls correctly. > > This partially reverts d6c4d3f2a69 which attempted the to do > the same thing, but with two calls to gen_set_rm(), which is > both inefficient and tickles an assertion in decode_save_opc. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1441 > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > --- > > > Reviewed-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com> > > target/riscv/helper.h | 1 + > target/riscv/fpu_helper.c | 37 +++++++++++++++++++++++++ > target/riscv/translate.c | 19 +++++++++++++ > target/riscv/insn_trans/trans_rvv.c.inc | 24 +++------------- > 4 files changed, 61 insertions(+), 20 deletions(-) > > diff --git a/target/riscv/helper.h b/target/riscv/helper.h > index 227c7122ef..9792ab5086 100644 > --- a/target/riscv/helper.h > +++ b/target/riscv/helper.h > @@ -3,6 +3,7 @@ DEF_HELPER_2(raise_exception, noreturn, env, i32) > > /* Floating Point - rounding mode */ > DEF_HELPER_FLAGS_2(set_rounding_mode, TCG_CALL_NO_WG, void, env, i32) > +DEF_HELPER_FLAGS_2(set_rounding_mode_chkfrm, TCG_CALL_NO_WG, void, env, i32) > DEF_HELPER_FLAGS_1(set_rod_rounding_mode, TCG_CALL_NO_WG, void, env) > > /* Floating Point - fused */ > diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c > index 5699c9517f..96817df8ef 100644 > --- a/target/riscv/fpu_helper.c > +++ b/target/riscv/fpu_helper.c > @@ -81,6 +81,43 @@ void helper_set_rounding_mode(CPURISCVState *env, uint32_t > rm) > set_float_rounding_mode(softrm, &env->fp_status); > } > > +void helper_set_rounding_mode_chkfrm(CPURISCVState *env, uint32_t rm) > +{ > + int softrm; > + > + /* Always validate frm, even if rm != DYN. */ > + if (unlikely(env->frm >= 5)) { > + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC()); > + } > + if (rm == RISCV_FRM_DYN) { > + rm = env->frm; > + } > + switch (rm) { > + case RISCV_FRM_RNE: > + softrm = float_round_nearest_even; > + break; > + case RISCV_FRM_RTZ: > + softrm = float_round_to_zero; > + break; > + case RISCV_FRM_RDN: > + softrm = float_round_down; > + break; > + case RISCV_FRM_RUP: > + softrm = float_round_up; > + break; > + case RISCV_FRM_RMM: > + softrm = float_round_ties_away; > + break; > + case RISCV_FRM_ROD: > + softrm = float_round_to_odd; > + break; > + default: > + g_assert_not_reached(); > + } > + > + set_float_rounding_mode(softrm, &env->fp_status); > +} > + > void helper_set_rod_rounding_mode(CPURISCVState *env) > { > set_float_rounding_mode(float_round_to_odd, &env->fp_status); > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index df38db7553..493c3815e1 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -114,6 +114,8 @@ typedef struct DisasContext { > bool pm_base_enabled; > /* Use icount trigger for native debug */ > bool itrigger; > + /* FRM is known to contain a valid value. */ > + bool frm_valid; > /* TCG of the current insn_start */ > TCGOp *insn_start; > } DisasContext; > @@ -674,12 +676,29 @@ static void gen_set_rm(DisasContext *ctx, int rm) > gen_helper_set_rod_rounding_mode(cpu_env); > return; > } > + if (rm == RISCV_FRM_DYN) { > + /* The helper will return only if frm valid. */ > + ctx->frm_valid = true; > + } > > /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */ > decode_save_opc(ctx); > gen_helper_set_rounding_mode(cpu_env, tcg_constant_i32(rm)); > } > > +static void gen_set_rm_chkfrm(DisasContext *ctx, int rm) > +{ > + if (ctx->frm == rm && ctx->frm_valid) { > + return; > + } > + ctx->frm = rm; > + ctx->frm_valid = true; > + > + /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */ > + decode_save_opc(ctx); > + gen_helper_set_rounding_mode_chkfrm(cpu_env, tcg_constant_i32(rm)); > +} > + > static int ex_plus_1(DisasContext *ctx, int nf) > { > return nf + 1; > diff --git a/target/riscv/insn_trans/trans_rvv.c.inc > b/target/riscv/insn_trans/trans_rvv.c.inc > index d455acedbf..bbb5c3a7b5 100644 > --- a/target/riscv/insn_trans/trans_rvv.c.inc > +++ b/target/riscv/insn_trans/trans_rvv.c.inc > @@ -2679,13 +2679,9 @@ static bool do_opfv(DisasContext *s, arg_rmr *a, > int rm) > { > if (checkfn(s, a)) { > - if (rm != RISCV_FRM_DYN) { > - gen_set_rm(s, RISCV_FRM_DYN); > - } > - > uint32_t data = 0; > TCGLabel *over = gen_new_label(); > - gen_set_rm(s, rm); > + gen_set_rm_chkfrm(s, rm); > tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over); > tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); > > @@ -2882,17 +2878,13 @@ static bool opffv_widen_check(DisasContext *s, > arg_rmr *a) > static bool trans_##NAME(DisasContext *s, arg_rmr *a) \ > { \ > if (CHECK(s, a)) { \ > - if (FRM != RISCV_FRM_DYN) { \ > - gen_set_rm(s, RISCV_FRM_DYN); \ > - } \ > - \ > uint32_t data = 0; \ > static gen_helper_gvec_3_ptr * const fns[2] = { \ > gen_helper_##HELPER##_h, \ > gen_helper_##HELPER##_w, \ > }; \ > TCGLabel *over = gen_new_label(); \ > - gen_set_rm(s, FRM); \ > + gen_set_rm_chkfrm(s, FRM); \ > tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over); \ > tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \ > \ > @@ -3005,17 +2997,13 @@ static bool opffv_narrow_check(DisasContext *s, > arg_rmr *a) > static bool trans_##NAME(DisasContext *s, arg_rmr *a) \ > { \ > if (CHECK(s, a)) { \ > - if (FRM != RISCV_FRM_DYN) { \ > - gen_set_rm(s, RISCV_FRM_DYN); \ > - } \ > - \ > uint32_t data = 0; \ > static gen_helper_gvec_3_ptr * const fns[2] = { \ > gen_helper_##HELPER##_h, \ > gen_helper_##HELPER##_w, \ > }; \ > TCGLabel *over = gen_new_label(); \ > - gen_set_rm(s, FRM); \ > + gen_set_rm_chkfrm(s, FRM); \ > tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over); \ > tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \ > \ > @@ -3060,10 +3048,6 @@ static bool opxfv_narrow_check(DisasContext *s, > arg_rmr *a) > static bool trans_##NAME(DisasContext *s, arg_rmr *a) \ > { \ > if (opxfv_narrow_check(s, a)) { \ > - if (FRM != RISCV_FRM_DYN) { \ > - gen_set_rm(s, RISCV_FRM_DYN); \ > - } \ > - \ > uint32_t data = 0; \ > static gen_helper_gvec_3_ptr * const fns[3] = { \ > gen_helper_##HELPER##_b, \ > @@ -3071,7 +3055,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmr *a) > \ > gen_helper_##HELPER##_w, \ > }; \ > TCGLabel *over = gen_new_label(); \ > - gen_set_rm(s, FRM); \ > + gen_set_rm_chkfrm(s, FRM); \ > tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over); \ > tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \ > \ > >
