Jakub Jelinek <ja...@redhat.com> writes:
> Hi!
>
> The following testcase is miscompiled on powerpc64le-linux starting with
> r15-6777.
> That change has the if (HONOR_NANS (GET_MODE (XEXP (op0, 0)))) all = 15;
> lines which work fine if the comparisons use MODE_FLOAT or MODE_INT operands
> (or say MODE_VECTOR* etc.).  But on this testcase on ppc64le during combine
> we see
> (set (reg:SI 134)
>     (ior:SI (ge:SI (reg:CCFP 128)
>             (const_int 0 [0]))
>         (lt:SI (reg:CCFP 128)
>             (const_int 0 [0]))))
> HONOR_NANS is obviously false on CCFPmode, because MODE_HAS_NANS is false,
> it isn't FLOAT_MODE_P.  But still it is a MODE_CC mode used for floating
> point comparisons and so we need to consider the possibility of unordered
> operands.
> I'm not sure how we could look at the setter of those MODE_CC regs
> from the simplifiers, after all they can happen in the middle of combiner
> trying to combine multiple instructions.
> So, instead the following patch attempts to be conservative for MODE_CC
> with some exceptions.  One is flag_finite_math_only, regardless of
> MODE_HAS_NANS in that case HONOR_NANS will be always false.
> Another one is for targets which provide REVERSE_CONDITION condition
> and reverse one way the floating point MODE_CC modes and another the
> integral ones.  If REVERSE_CONDITION for GT gives LE, then unordered
> is not an option.  And finally it searches if there are any scalar floating
> point modes with MODE_HAS_NANS at all, if not, it is also safe to
> assume there are no NaNs.
>
> Bootstrapped/regtested on x86_64-linux, i686-linux, powerpc64le-linux,
> aarch64-linux and bootstrapped on s390x-linux (regtest there still pending).
>
> Ok for trunk?
>
> Or any other ideas how to handle this?

I think we should instead go back to punting on comparisons whose inputs
are CC modes, as we did (indirectly, via comparison_code_valid_for_mode)
before r15-6777.  Sorry, I'd forgotten/hadn't thought to exclude CC modes
explicitly when removing that function.

Richard

>
> 2025-02-24  Jakub Jelinek  <ja...@redhat.com>
>
>       PR rtl-optimization/119002
>       * simplify-rtx.cc: Include tm_p.h.
>       (simplify_context::simplify_logical_relational_operation): Set
>       all = 15 also if op0's first operand has MODE_CC mode and it
>       is or could be floating point comparison which honors NaNs.
>
>       * gcc.c-torture/execute/ieee/pr119002.c: New test.
>
> --- gcc/simplify-rtx.cc.jj    2025-01-15 08:43:39.611918569 +0100
> +++ gcc/simplify-rtx.cc       2025-02-24 21:16:09.980758481 +0100
> @@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.
>  #include "selftest-rtl.h"
>  #include "rtx-vector-builder.h"
>  #include "rtlanal.h"
> +#include "tm_p.h"
>  
>  /* Simplification and canonicalization of RTL.  */
>  
> @@ -2675,6 +2676,24 @@ simplify_context::simplify_logical_relat
>        /* See whether the operands might be unordered.  */
>        if (HONOR_NANS (GET_MODE (XEXP (op0, 0))))
>       all = 15;
> +      else if (GET_MODE_CLASS (GET_MODE (XEXP (op0, 0))) == MODE_CC
> +            && !flag_finite_math_only)
> +     {
> +       /* HONOR_NANS will be false for MODE_CC comparisons, eventhough
> +          they could actually be floating point.  If the mode is
> +          reversible, ask the backend if it could be unordered, otherwise
> +          err on the side of caution and assume it could be unordered
> +          if any supported floating mode honors NaNs.  */
> +       machine_mode mode = GET_MODE (XEXP (op0, 0));
> +       if (!REVERSIBLE_CC_MODE (mode)
> +           || REVERSE_CONDITION (GT, mode) != LE)
> +         FOR_EACH_MODE_IN_CLASS (mode, MODE_FLOAT)
> +           if (HONOR_NANS (mode))
> +             {
> +               all = 15;
> +               break;
> +             }
> +     }
>        mask0 = comparison_to_mask (code0) & all;
>        mask1 = comparison_to_mask (code1) & all;
>      }
>
> --- gcc/testsuite/gcc.c-torture/execute/ieee/pr119002.c.jj    2025-02-24 
> 21:18:45.880622627 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/ieee/pr119002.c       2025-02-24 
> 21:19:02.418396051 +0100
> @@ -0,0 +1,23 @@
> +/* PR rtl-optimization/119002 */
> +
> +__attribute__((noipa)) unsigned int
> +foo (void *x, float y, float z)
> +{
> +  unsigned int a, b;
> +  float c, d, e;
> +  c = y;
> +  d = z;
> +  a = c < d;
> +  d = y;
> +  e = z;
> +  b = d >= e;
> +  a |= b;
> +  return a;
> +}
> +
> +int
> +main ()
> +{
> +  if (foo ((void *) 0, 0.f, __builtin_nanf ("")))
> +    __builtin_abort ();
> +}
>
>       Jakub

Reply via email to