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
