Re: [PULL 29/31] target/i386: Use correct type for get_float_exception_flags() values

On 6/6/25 15:34, Paolo Bonzini wrote:
> From: Peter Maydell <[email protected]>
> 
> The softfloat get_float_exception_flags() function returns 'int', but
> in various places in target/i386 we incorrectly store the returned
> value into a uint8_t.  This currently has no ill effects because i386
> doesn't care about any of the float_flag enum values above 0x40.
> However, we want to start using float_flag_input_denormal_used, which
> is 0x4000.
> 
> Switch to using 'int' so that we can handle all the possible valid
> float_flag_* values. This includes changing the return type of
> save_exception_flags() and the argument to merge_exception_flags().
> 
> Signed-off-by: Peter Maydell <[email protected]>
> Reviewed-by: Richard Henderson <[email protected]>
> Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
> Reviewed-by: Zhao Liu <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
>   target/i386/ops_sse.h        | 16 +++----
>   target/i386/tcg/fpu_helper.c | 82 ++++++++++++++++++------------------
>   2 files changed, 49 insertions(+), 49 deletions(-)

I'm picking this small change up for 10.0.x stable series, -- a subsequent
patch in this area touches the same places, and it looks like it's better
to pick up this change instead of editing subsequent changes.  This one
looks harmless to me.

Please let me know if I shoudln't.

Thanks,

/mjt

> diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
> index f0aa1894aa2..a2e4d480399 100644
> --- a/target/i386/ops_sse.h
> +++ b/target/i386/ops_sse.h
> @@ -842,7 +842,7 @@ int64_t helper_cvttsd2sq(CPUX86State *env, ZMMReg *s)
>   
>   void glue(helper_rsqrtps, SUFFIX)(CPUX86State *env, ZMMReg *d, ZMMReg *s)
>   {
> -    uint8_t old_flags = get_float_exception_flags(&env->sse_status);
> +    int old_flags = get_float_exception_flags(&env->sse_status);
>       int i;
>       for (i = 0; i < 2 << SHIFT; i++) {
>           d->ZMM_S(i) = float32_div(float32_one,
> @@ -855,7 +855,7 @@ void glue(helper_rsqrtps, SUFFIX)(CPUX86State *env, ZMMReg *d, ZMMReg *s)
>   #if SHIFT == 1
>   void helper_rsqrtss(CPUX86State *env, ZMMReg *d, ZMMReg *v, ZMMReg *s)
>   {
> -    uint8_t old_flags = get_float_exception_flags(&env->sse_status);
> +    int old_flags = get_float_exception_flags(&env->sse_status);
>       int i;
>       d->ZMM_S(0) = float32_div(float32_one,
>                                 float32_sqrt(s->ZMM_S(0), &env->sse_status),
> @@ -869,7 +869,7 @@ void helper_rsqrtss(CPUX86State *env, ZMMReg *d, ZMMReg *v, ZMMReg *s)
>   
>   void glue(helper_rcpps, SUFFIX)(CPUX86State *env, ZMMReg *d, ZMMReg *s)
>   {
> -    uint8_t old_flags = get_float_exception_flags(&env->sse_status);
> +    int old_flags = get_float_exception_flags(&env->sse_status);
>       int i;
>       for (i = 0; i < 2 << SHIFT; i++) {
>           d->ZMM_S(i) = float32_div(float32_one, s->ZMM_S(i), &env->sse_status);
> @@ -880,7 +880,7 @@ void glue(helper_rcpps, SUFFIX)(CPUX86State *env, ZMMReg *d, ZMMReg *s)
>   #if SHIFT == 1
>   void helper_rcpss(CPUX86State *env, ZMMReg *d, ZMMReg *v, ZMMReg *s)
>   {
> -    uint8_t old_flags = get_float_exception_flags(&env->sse_status);
> +    int old_flags = get_float_exception_flags(&env->sse_status);
>       int i;
>       d->ZMM_S(0) = float32_div(float32_one, s->ZMM_S(0), &env->sse_status);
>       for (i = 1; i < 2 << SHIFT; i++) {
> @@ -1714,7 +1714,7 @@ void glue(helper_phminposuw, SUFFIX)(CPUX86State *env, Reg *d, Reg *s)
>   void glue(helper_roundps, SUFFIX)(CPUX86State *env, Reg *d, Reg *s,
>                                     uint32_t mode)
>   {
> -    uint8_t old_flags = get_float_exception_flags(&env->sse_status);
> +    int old_flags = get_float_exception_flags(&env->sse_status);
>       signed char prev_rounding_mode;
>       int i;
>   
> @@ -1738,7 +1738,7 @@ void glue(helper_roundps, SUFFIX)(CPUX86State *env, Reg *d, Reg *s,
>   void glue(helper_roundpd, SUFFIX)(CPUX86State *env, Reg *d, Reg *s,
>                                     uint32_t mode)
>   {
> -    uint8_t old_flags = get_float_exception_flags(&env->sse_status);
> +    int old_flags = get_float_exception_flags(&env->sse_status);
>       signed char prev_rounding_mode;
>       int i;
>   
> @@ -1763,7 +1763,7 @@ void glue(helper_roundpd, SUFFIX)(CPUX86State *env, Reg *d, Reg *s,
>   void glue(helper_roundss, SUFFIX)(CPUX86State *env, Reg *d, Reg *v, Reg *s,
>                                     uint32_t mode)
>   {
> -    uint8_t old_flags = get_float_exception_flags(&env->sse_status);
> +    int old_flags = get_float_exception_flags(&env->sse_status);
>       signed char prev_rounding_mode;
>       int i;
>   
> @@ -1788,7 +1788,7 @@ void glue(helper_roundss, SUFFIX)(CPUX86State *env, Reg *d, Reg *v, Reg *s,
>   void glue(helper_roundsd, SUFFIX)(CPUX86State *env, Reg *d, Reg *v, Reg *s,
>                                     uint32_t mode)
>   {
> -    uint8_t old_flags = get_float_exception_flags(&env->sse_status);
> +    int old_flags = get_float_exception_flags(&env->sse_status);
>       signed char prev_rounding_mode;
>       int i;
>   
> diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
> index 9ea67ea76c8..4732b718129 100644
> --- a/target/i386/tcg/fpu_helper.c
> +++ b/target/i386/tcg/fpu_helper.c
> @@ -198,16 +198,16 @@ void cpu_init_fp_statuses(CPUX86State *env)
>       set_float_ftz_detection(float_ftz_after_rounding, &env->sse_status);
>   }
>   
> -static inline uint8_t save_exception_flags(CPUX86State *env)
> +static inline int save_exception_flags(CPUX86State *env)
>   {
> -    uint8_t old_flags = get_float_exception_flags(&env->fp_status);
> +    int old_flags = get_float_exception_flags(&env->fp_status);
>       set_float_exception_flags(0, &env->fp_status);
>       return old_flags;
>   }
>   
> -static void merge_exception_flags(CPUX86State *env, uint8_t old_flags)
> +static void merge_exception_flags(CPUX86State *env, int old_flags)
>   {
> -    uint8_t new_flags = get_float_exception_flags(&env->fp_status);
> +    int new_flags = get_float_exception_flags(&env->fp_status);
>       float_raise(old_flags, &env->fp_status);
>       fpu_set_exception(env,
>                         ((new_flags & float_flag_invalid ? FPUS_IE : 0) |
> @@ -220,7 +220,7 @@ static void merge_exception_flags(CPUX86State *env, uint8_t old_flags)
>   
>   static inline floatx80 helper_fdiv(CPUX86State *env, floatx80 a, floatx80 b)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       floatx80 ret = floatx80_div(a, b, &env->fp_status);
>       merge_exception_flags(env, old_flags);
>       return ret;
> @@ -240,7 +240,7 @@ static void fpu_raise_exception(CPUX86State *env, uintptr_t retaddr)
>   
>   void helper_flds_FT0(CPUX86State *env, uint32_t val)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       union {
>           float32 f;
>           uint32_t i;
> @@ -253,7 +253,7 @@ void helper_flds_FT0(CPUX86State *env, uint32_t val)
>   
>   void helper_fldl_FT0(CPUX86State *env, uint64_t val)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       union {
>           float64 f;
>           uint64_t i;
> @@ -271,7 +271,7 @@ void helper_fildl_FT0(CPUX86State *env, int32_t val)
>   
>   void helper_flds_ST0(CPUX86State *env, uint32_t val)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       int new_fpstt;
>       union {
>           float32 f;
> @@ -288,7 +288,7 @@ void helper_flds_ST0(CPUX86State *env, uint32_t val)
>   
>   void helper_fldl_ST0(CPUX86State *env, uint64_t val)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       int new_fpstt;
>       union {
>           float64 f;
> @@ -338,7 +338,7 @@ void helper_fildll_ST0(CPUX86State *env, int64_t val)
>   
>   uint32_t helper_fsts_ST0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       union {
>           float32 f;
>           uint32_t i;
> @@ -351,7 +351,7 @@ uint32_t helper_fsts_ST0(CPUX86State *env)
>   
>   uint64_t helper_fstl_ST0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       union {
>           float64 f;
>           uint64_t i;
> @@ -364,7 +364,7 @@ uint64_t helper_fstl_ST0(CPUX86State *env)
>   
>   int32_t helper_fist_ST0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       int32_t val;
>   
>       val = floatx80_to_int32(ST0, &env->fp_status);
> @@ -378,7 +378,7 @@ int32_t helper_fist_ST0(CPUX86State *env)
>   
>   int32_t helper_fistl_ST0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       int32_t val;
>   
>       val = floatx80_to_int32(ST0, &env->fp_status);
> @@ -391,7 +391,7 @@ int32_t helper_fistl_ST0(CPUX86State *env)
>   
>   int64_t helper_fistll_ST0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       int64_t val;
>   
>       val = floatx80_to_int64(ST0, &env->fp_status);
> @@ -404,7 +404,7 @@ int64_t helper_fistll_ST0(CPUX86State *env)
>   
>   int32_t helper_fistt_ST0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       int32_t val;
>   
>       val = floatx80_to_int32_round_to_zero(ST0, &env->fp_status);
> @@ -418,7 +418,7 @@ int32_t helper_fistt_ST0(CPUX86State *env)
>   
>   int32_t helper_fisttl_ST0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       int32_t val;
>   
>       val = floatx80_to_int32_round_to_zero(ST0, &env->fp_status);
> @@ -431,7 +431,7 @@ int32_t helper_fisttl_ST0(CPUX86State *env)
>   
>   int64_t helper_fisttll_ST0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       int64_t val;
>   
>       val = floatx80_to_int64_round_to_zero(ST0, &env->fp_status);
> @@ -527,7 +527,7 @@ static const int fcom_ccval[4] = {0x0100, 0x4000, 0x0000, 0x4500};
>   
>   void helper_fcom_ST0_FT0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       FloatRelation ret;
>   
>       ret = floatx80_compare(ST0, FT0, &env->fp_status);
> @@ -537,7 +537,7 @@ void helper_fcom_ST0_FT0(CPUX86State *env)
>   
>   void helper_fucom_ST0_FT0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       FloatRelation ret;
>   
>       ret = floatx80_compare_quiet(ST0, FT0, &env->fp_status);
> @@ -549,7 +549,7 @@ static const int fcomi_ccval[4] = {CC_C, CC_Z, 0, CC_Z | CC_P | CC_C};
>   
>   void helper_fcomi_ST0_FT0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       int eflags;
>       FloatRelation ret;
>   
> @@ -562,7 +562,7 @@ void helper_fcomi_ST0_FT0(CPUX86State *env)
>   
>   void helper_fucomi_ST0_FT0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       int eflags;
>       FloatRelation ret;
>   
> @@ -575,28 +575,28 @@ void helper_fucomi_ST0_FT0(CPUX86State *env)
>   
>   void helper_fadd_ST0_FT0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       ST0 = floatx80_add(ST0, FT0, &env->fp_status);
>       merge_exception_flags(env, old_flags);
>   }
>   
>   void helper_fmul_ST0_FT0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       ST0 = floatx80_mul(ST0, FT0, &env->fp_status);
>       merge_exception_flags(env, old_flags);
>   }
>   
>   void helper_fsub_ST0_FT0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       ST0 = floatx80_sub(ST0, FT0, &env->fp_status);
>       merge_exception_flags(env, old_flags);
>   }
>   
>   void helper_fsubr_ST0_FT0(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       ST0 = floatx80_sub(FT0, ST0, &env->fp_status);
>       merge_exception_flags(env, old_flags);
>   }
> @@ -615,28 +615,28 @@ void helper_fdivr_ST0_FT0(CPUX86State *env)
>   
>   void helper_fadd_STN_ST0(CPUX86State *env, int st_index)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       ST(st_index) = floatx80_add(ST(st_index), ST0, &env->fp_status);
>       merge_exception_flags(env, old_flags);
>   }
>   
>   void helper_fmul_STN_ST0(CPUX86State *env, int st_index)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       ST(st_index) = floatx80_mul(ST(st_index), ST0, &env->fp_status);
>       merge_exception_flags(env, old_flags);
>   }
>   
>   void helper_fsub_STN_ST0(CPUX86State *env, int st_index)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       ST(st_index) = floatx80_sub(ST(st_index), ST0, &env->fp_status);
>       merge_exception_flags(env, old_flags);
>   }
>   
>   void helper_fsubr_STN_ST0(CPUX86State *env, int st_index)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       ST(st_index) = floatx80_sub(ST0, ST(st_index), &env->fp_status);
>       merge_exception_flags(env, old_flags);
>   }
> @@ -861,7 +861,7 @@ void helper_fbld_ST0(CPUX86State *env, target_ulong ptr)
>   
>   void helper_fbst_ST0(CPUX86State *env, target_ulong ptr)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       int v;
>       target_ulong mem_ref, mem_end;
>       int64_t val;
> @@ -1136,7 +1136,7 @@ static const struct f2xm1_data f2xm1_table[65] = {
>   
>   void helper_f2xm1(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       uint64_t sig = extractFloatx80Frac(ST0);
>       int32_t exp = extractFloatx80Exp(ST0);
>       bool sign = extractFloatx80Sign(ST0);
> @@ -1369,7 +1369,7 @@ static const struct fpatan_data fpatan_table[9] = {
>   
>   void helper_fpatan(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       uint64_t arg0_sig = extractFloatx80Frac(ST0);
>       int32_t arg0_exp = extractFloatx80Exp(ST0);
>       bool arg0_sign = extractFloatx80Sign(ST0);
> @@ -1808,7 +1808,7 @@ void helper_fpatan(CPUX86State *env)
>   
>   void helper_fxtract(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       CPU_LDoubleU temp;
>   
>       temp.d = ST0;
> @@ -1857,7 +1857,7 @@ void helper_fxtract(CPUX86State *env)
>   
>   static void helper_fprem_common(CPUX86State *env, bool mod)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       uint64_t quotient;
>       CPU_LDoubleU temp0, temp1;
>       int exp0, exp1, expdiff;
> @@ -2053,7 +2053,7 @@ static void helper_fyl2x_common(CPUX86State *env, floatx80 arg, int32_t *exp,
>   
>   void helper_fyl2xp1(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       uint64_t arg0_sig = extractFloatx80Frac(ST0);
>       int32_t arg0_exp = extractFloatx80Exp(ST0);
>       bool arg0_sign = extractFloatx80Sign(ST0);
> @@ -2151,7 +2151,7 @@ void helper_fyl2xp1(CPUX86State *env)
>   
>   void helper_fyl2x(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       uint64_t arg0_sig = extractFloatx80Frac(ST0);
>       int32_t arg0_exp = extractFloatx80Exp(ST0);
>       bool arg0_sign = extractFloatx80Sign(ST0);
> @@ -2298,7 +2298,7 @@ void helper_fyl2x(CPUX86State *env)
>   
>   void helper_fsqrt(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       if (floatx80_is_neg(ST0)) {
>           env->fpus &= ~0x4700;  /* (C3,C2,C1,C0) <-- 0000 */
>           env->fpus |= 0x400;
> @@ -2324,14 +2324,14 @@ void helper_fsincos(CPUX86State *env)
>   
>   void helper_frndint(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       ST0 = floatx80_round_to_int(ST0, &env->fp_status);
>       merge_exception_flags(env, old_flags);
>   }
>   
>   void helper_fscale(CPUX86State *env)
>   {
> -    uint8_t old_flags = save_exception_flags(env);
> +    int old_flags = save_exception_flags(env);
>       if (floatx80_invalid_encoding(ST1, &env->fp_status) ||
>           floatx80_invalid_encoding(ST0, &env->fp_status)) {
>           float_raise(float_flag_invalid, &env->fp_status);
> @@ -2369,7 +2369,7 @@ void helper_fscale(CPUX86State *env)
>       } else {
>           int n;
>           FloatX80RoundPrec save = env->fp_status.floatx80_rounding_precision;
> -        uint8_t save_flags = get_float_exception_flags(&env->fp_status);
> +        int save_flags = get_float_exception_flags(&env->fp_status);
>           set_float_exception_flags(0, &env->fp_status);
>           n = floatx80_to_int32_round_to_zero(ST1, &env->fp_status);
>           set_float_exception_flags(save_flags, &env->fp_status);
> @@ -3269,7 +3269,7 @@ void update_mxcsr_status(CPUX86State *env)
>   
>   void update_mxcsr_from_sse_status(CPUX86State *env)
>   {
> -    uint8_t flags = get_float_exception_flags(&env->sse_status);
> +    int flags = get_float_exception_flags(&env->sse_status);
>       /*
>        * The MXCSR denormal flag has opposite semantics to
>        * float_flag_input_denormal_flushed (the softfloat code sets that flag