Re: [PATCH] ppc/translate: Fix unordered f64/f128 comparisons

[Richard Henderson]

On 11/9/20 2:21 AM, LemonBoy wrote:
> According to the PowerISA v3.1 reference, Table 68 "Actions for xscmpudp
> - Part 1: Compare Unordered", whenever one of the two operands is a NaN
> the SO bit is set while the other three bits are cleared.
> 
> Apply the same change to xscmpuqp.
> 
> The respective ordered counterparts are unaffected.
> 
> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
> ---
>  target/ppc/fpu_helper.c | 32 ++++++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
> index 9b8c8b70b6..b07ff66375 100644
> --- a/target/ppc/fpu_helper.c
> +++ b/target/ppc/fpu_helper.c
> @@ -2479,13 +2479,11 @@ void helper_##op(CPUPPCState *env, uint32_t opcode,   
>                    \
>      if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status) ||        \
>          float64_is_signaling_nan(xb->VsrD(0), &env->fp_status)) {        \
>          vxsnan_flag = true;                                              \
> -        cc = CRF_SO;                                                     \
>          if (fpscr_ve == 0 && ordered) {                                  \
>              vxvc_flag = true;                                            \
>          }                                                                \
>      } else if (float64_is_quiet_nan(xa->VsrD(0), &env->fp_status) ||     \
>                 float64_is_quiet_nan(xb->VsrD(0), &env->fp_status)) {     \
> -        cc = CRF_SO;                                                     \
>          if (ordered) {                                                   \
>              vxvc_flag = true;                                            \
>          }                                                                \
> @@ -2497,12 +2495,19 @@ void helper_##op(CPUPPCState *env, uint32_t opcode,   
>                    \
>          float_invalid_op_vxvc(env, 0, GETPC());                          \
>      }                                                                    \
>                                                                           \
> -    if (float64_lt(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {         \
> +    switch (float64_compare(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {\
> +    case float_relation_less:                                            \
>          cc |= CRF_LT;                                                    \
> -    } else if (!float64_le(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) { \
> -        cc |= CRF_GT;                                                    \
> -    } else {                                                             \
> +        break;                                                           \
> +    case float_relation_equal:                                           \
>          cc |= CRF_EQ;                                                    \
> +        break;                                                           \
> +    case float_relation_greater:                                         \
> +        cc |= CRF_GT;                                                    \
> +        break;                                                           \
> +    case float_relation_unordered:                                       \
> +        cc |= CRF_SO;                                                    \
> +        break;                                                           \
>      }                          

This needs some more cleanup.  There's no point in checking for nans first;
wait until you get to float_relation_unordered.

These macros should be made into straight functions.


r~