| Message ID | 20241202131347.498124-30-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fpu: Remove pickNaNMulAdd, default-NaN ifdefs | expand |
On 12/2/24 07:13, Peter Maydell wrote: > In target/loongarch's helper_fclass_s() and helper_fclass_d() we pass > a zero-initialized float_status struct to float32_is_quiet_nan() and > float64_is_quiet_nan(), with the cryptic comment "for > snan_bit_is_one". > > This pattern appears to have been copied from target/riscv, where it > is used because the functions there do not have ready access to the > CPU state struct. The comment presumably refers to the fact that the > main reason the is_quiet_nan() functions want the float_state is > because they want to know about the snan_bit_is_one config. > > In the loongarch helpers, though, we have the CPU state struct > to hand. Use the usual env->fp_status here. This avoids our needing > to track that we need to update the initializer of the local > float_status structs when the core softfloat code adds new > options for targets to configure their behaviour. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > It would be nice to get the riscv fclass_s etc also not to use a > stunt float_status, but plumbing the env through would require > fiddling with the macro magic, and in practice for the is_quiet_nan > functions it works out OK. > --- > target/loongarch/tcg/fpu_helper.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/loongarch/tcg/fpu_helper.c b/target/loongarch/tcg/fpu_helper.c index 37a48599366..aea5e0fe5e6 100644 --- a/target/loongarch/tcg/fpu_helper.c +++ b/target/loongarch/tcg/fpu_helper.c @@ -359,8 +359,7 @@ uint64_t helper_fclass_s(CPULoongArchState *env, uint64_t fj) } else if (float32_is_zero_or_denormal(f)) { return sign ? 1 << 4 : 1 << 8; } else if (float32_is_any_nan(f)) { - float_status s = { }; /* for snan_bit_is_one */ - return float32_is_quiet_nan(f, &s) ? 1 << 1 : 1 << 0; + return float32_is_quiet_nan(f, &env->fp_status) ? 1 << 1 : 1 << 0; } else { return sign ? 1 << 3 : 1 << 7; } @@ -378,8 +377,7 @@ uint64_t helper_fclass_d(CPULoongArchState *env, uint64_t fj) } else if (float64_is_zero_or_denormal(f)) { return sign ? 1 << 4 : 1 << 8; } else if (float64_is_any_nan(f)) { - float_status s = { }; /* for snan_bit_is_one */ - return float64_is_quiet_nan(f, &s) ? 1 << 1 : 1 << 0; + return float64_is_quiet_nan(f, &env->fp_status) ? 1 << 1 : 1 << 0; } else { return sign ? 1 << 3 : 1 << 7; }
In target/loongarch's helper_fclass_s() and helper_fclass_d() we pass a zero-initialized float_status struct to float32_is_quiet_nan() and float64_is_quiet_nan(), with the cryptic comment "for snan_bit_is_one". This pattern appears to have been copied from target/riscv, where it is used because the functions there do not have ready access to the CPU state struct. The comment presumably refers to the fact that the main reason the is_quiet_nan() functions want the float_state is because they want to know about the snan_bit_is_one config. In the loongarch helpers, though, we have the CPU state struct to hand. Use the usual env->fp_status here. This avoids our needing to track that we need to update the initializer of the local float_status structs when the core softfloat code adds new options for targets to configure their behaviour. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- It would be nice to get the riscv fclass_s etc also not to use a stunt float_status, but plumbing the env through would require fiddling with the macro magic, and in practice for the is_quiet_nan functions it works out OK. --- target/loongarch/tcg/fpu_helper.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)