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