[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] target/arm: Introduce helper_set_rounding_mode_chkfrm
|
From: |
Alistair Francis |
|
Subject: |
Re: [PATCH 1/2] target/arm: Introduce helper_set_rounding_mode_chkfrm |
|
Date: |
Thu, 19 Jan 2023 08:51:56 +1000 |
On Mon, Jan 16, 2023 at 2:08 AM Richard Henderson
<richard.henderson@linaro.org> 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.henderson@linaro.org>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> 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); \
> \
> --
> 2.34.1
>
>