On 5/23/23 16:33, Richard Henderson wrote:
The tests are poorly ordered, testing many unlikely things before the
most likely thing (normal). A better ordering would be
if (likely(tp##_is_normal(arg))) {
} else if (tp##_is_zero(arg)) {
} else if (tp##_is_zero_or_denormal(arg)) {
} else if (tp##_is_infinity(arg)) {
} else {
// nan case
}
Secondly, we compute the classify bitmask, and then deconstruct the mask
again in set_fprf_from_class. Since we don't use the classify bitmask
for anything else, better would be to compute the fprf value directly in
the if-ladder.
So something like this:
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index a66e16c2128c..daed97ca178e 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -141,62 +141,30 @@ static inline int ppc_float64_get_unbiased_exp(float64 f)
return ((f >> 52) & 0x7FF) - 1023;
}
-/* Classify a floating-point number. */
-enum {
- is_normal = 1,
- is_zero = 2,
- is_denormal = 4,
- is_inf = 8,
- is_qnan = 16,
- is_snan = 32,
- is_neg = 64,
-};
-
-#define COMPUTE_CLASS(tp) \
-static int tp##_classify(tp arg) \
-{ \
- int ret = tp##_is_neg(arg) * is_neg; \
- if (unlikely(tp##_is_any_nan(arg))) { \
- float_status dummy = { }; /* snan_bit_is_one = 0 */ \
- ret |= (tp##_is_signaling_nan(arg, &dummy) \
- ? is_snan : is_qnan); \
- } else if (unlikely(tp##_is_infinity(arg))) { \
- ret |= is_inf; \
- } else if (tp##_is_zero(arg)) { \
- ret |= is_zero; \
- } else if (tp##_is_zero_or_denormal(arg)) { \
- ret |= is_denormal; \
- } else { \
- ret |= is_normal; \
- } \
- return ret; \
-}
-
-COMPUTE_CLASS(float16)
-COMPUTE_CLASS(float32)
-COMPUTE_CLASS(float64)
-COMPUTE_CLASS(float128)
-
-static void set_fprf_from_class(CPUPPCState *env, int class)
+static void set_fprf(CPUPPCState *env, uint8_t ret)
{
- static const uint8_t fprf[6][2] = {
- { 0x04, 0x08 }, /* normalized */
- { 0x02, 0x12 }, /* zero */
- { 0x14, 0x18 }, /* denormalized */
- { 0x05, 0x09 }, /* infinity */
- { 0x11, 0x11 }, /* qnan */
- { 0x00, 0x00 }, /* snan -- flags are undefined */
- };
- bool isneg = class & is_neg;
-
env->fpscr &= ~FP_FPRF;
- env->fpscr |= fprf[ctz32(class)][isneg] << FPSCR_FPRF;
+ env->fpscr |= ret << FPSCR_FPRF;
}
-#define COMPUTE_FPRF(tp) \
-void helper_compute_fprf_##tp(CPUPPCState *env, tp arg) \
-{ \
- set_fprf_from_class(env, tp##_classify(arg)); \
+#define COMPUTE_FPRF(tp) \
+void helper_compute_fprf_##tp(CPUPPCState *env, tp arg) \
+{ \
+ int ret; \
+ if (tp##_is_normal(arg)) { \
+ ret = 0x0408; \
+ } else if (tp##_is_zero(arg)) { \
+ ret = 0x0212; \
+ } else if (tp##_is_zero_or_denormal(arg)) { \
+ ret = 0x1418; \
+ } else if (unlikely(tp##_is_infinity(arg))) { \
+ ret = 0x0509; \
+ } else { \
+ float_status dummy = { }; /* snan_bit_is_one = 0 */ \
+ ret = (tp##_is_signaling_nan(arg, &dummy) \
+ ? 0x0000 : 0x1111); \
+ } \
+ set_fprf(env, tp##_is_neg(arg) ? (uint8_t)ret : ret >> 8); \
}
COMPUTE_FPRF(float16)
Not tested beyond compilation, but if Zoltan reports that it helps
I can write a commit message and submit it.
Paolo
