From 220a58d9abbb1f403e8f79cd42ad01b7c9b10ae9 Mon Sep 17 00:00:00 2001 From: Aldy Hernandez Date: Mon, 18 Sep 2023 21:41:08 -0400 Subject: [PATCH] [frange] Clean up floating point relational folding. The following patch removes all the special casing from the floating point relational folding code. Now all the code relating to folding of relationals is in frelop_early_resolve() and in operator_not_equal::fold_range() which requires a small tweak. I have written new relational tests, and moved them to gcc.dg/tree-ssa/vrp-float-relations-* for easy reference. In the tests it's easy to see the type of things we need to handle: (a) if (x != y) if (x == y) link_error (); (b) if (a != b) if (a != b) // Foldable as true. (c) /* We can thread BB2->BB4->BB5 even though we have no knowledge of the NANness of either x_1 or a_5. */ __BB(4): x_1 = __PHI (__BB2: a_5(D), __BB3: b_4(D)); if (x_1 __UNEQ a_5(D)) (d) /* Even though x_1 and a_4 are equivalent on the BB2->BB4 path, we cannot fold the conditional because of possible NANs: */ __BB(4): # x_1 = __PHI (__BB2: a_4(D), __BB3: 8.0e+0(3)); if (x_1 == a_4(D)) (e) if (cond) x = a; else x = 8.0; /* We can fold this as false on the path coming out of cond==1, regardless of NANs on either "x" or "a". */ if (x < a) stuff (); [etc, etc] We can implement everything without either special casing, get_identity_relation(), or adding new unordered relationals. The basic idea is that if we accurately reflect NANs in op[12]_range, this information gets propagated to the relevant edges, and there's no need for unordered relations (VREL_UN*), because the information is in the range itself. This information is then used in frelop_early_resolve() to fold certain combinations. I don't mean this patch as a hard-no against implementing the unordered relations Jakub preferred, but seeing that it's looking cleaner and trivially simple without the added burden of more enums, I'd like to flesh it out completely and then discuss if we still think new codes are needed. More testcases or corner cases are highly welcome. In follow-up patches I will finish up unordered relation folding, and come up with suitable tests. gcc/ChangeLog: * range-op-float.cc (frelop_early_resolve): Clean-up and remove special casing. (operator_not_equal::fold_range): Handle VREL_EQ. (operator_lt::fold_range): Remove special casing for VREL_EQ. (operator_gt::fold_range): Same. (foperator_unordered_equal::fold_range): Same. gcc/testsuite/ChangeLog: * gcc.dg/tree-ssa/vrp-float-12.c: Moved to... * gcc.dg/tree-ssa/vrp-float-relations-1.c: ...here. * gcc.dg/tree-ssa/vrp-float-relations-2.c: New test. * gcc.dg/tree-ssa/vrp-float-relations-3.c: New test. * gcc.dg/tree-ssa/vrp-float-relations-4.c: New test. --- gcc/range-op-float.cc | 51 ++++--------------- ...vrp-float-12.c => vrp-float-relations-1.c} | 0 .../gcc.dg/tree-ssa/vrp-float-relations-2.c | 21 ++++++++ .../gcc.dg/tree-ssa/vrp-float-relations-3.c | 27 ++++++++++ .../gcc.dg/tree-ssa/vrp-float-relations-4.c | 38 ++++++++++++++ 5 files changed, 95 insertions(+), 42 deletions(-) rename gcc/testsuite/gcc.dg/tree-ssa/{vrp-float-12.c => vrp-float-relations-1.c} (100%) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-2.c create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-3.c create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-4.c diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc index 5eb1d9c06e3..399deee5d8a 100644 --- a/gcc/range-op-float.cc +++ b/gcc/range-op-float.cc @@ -281,18 +281,6 @@ maybe_isnan (const frange &op1, const frange &op2) // folded. For example, when attempting to fold x_3 == y_5, MY_REL is // VREL_EQ, and if the statement is dominated by x_3 > y_5, then // TRIO.op1_op2() is VREL_GT. -// -// Relations in a floating point world are a bit tricky, as TRIO -// behaves as the corresponding unordered variant if either operand -// could be a NAN. For example, when resolving "if (x_5 == x_5)", the -// relation is VREL_EQ, but it behaves like VREL_UNEQ if NANs are a -// possibility. Similarly, the false edge of "if (x >= y)" has a -// relation of VREL_LT, but behaves as VREL_UNLT unless we can prove -// neither operand can be NAN. -// -// ?? It is unclear whether providing unordered VREL relations would -// simplify things, as we'd have to add more entries to various -// tables, tweak all the op1_op2_relation() entries, etc. static inline bool frelop_early_resolve (irange &r, tree type, @@ -301,14 +289,6 @@ frelop_early_resolve (irange &r, tree type, { relation_kind rel = trio.op1_op2 (); - if (maybe_isnan (op1, op2)) - { - // There's not much we can do for VREL_EQ if NAN is a - // possibility. It is up to the caller to deal with these - // special cases. - if (rel == VREL_EQ) - return empty_range_varying (r, type, op1, op2); - } // If known relation is a complete subset of this relation, always // return true. However, avoid doing this when NAN is a possibility // as we'll incorrectly fold conditions: @@ -319,7 +299,7 @@ frelop_early_resolve (irange &r, tree type, // ;; With NANs the relation here is basically VREL_UNLT, so we // ;; can't fold the following: // if (x_3 < y_5) - else if (relation_union (rel, my_rel) == my_rel) + if (!maybe_isnan (op1, op2) && relation_union (rel, my_rel) == my_rel) { r = range_true (type); return true; @@ -801,7 +781,13 @@ operator_not_equal::fold_range (irange &r, tree type, r = range_true (type); return true; } - if (frelop_early_resolve (r, type, op1, op2, trio, VREL_NE)) + if (rel == VREL_EQ && maybe_isnan (op1, op2)) + { + // Avoid frelop_early_resolve() below as it could fold to FALSE + // without regards to NANs. This would be incorrect if trying + // to fold x_5 != x_5 without prior knowledge of NANs. + } + else if (frelop_early_resolve (r, type, op1, op2, trio, VREL_NE)) return true; // x != NAN is always TRUE. @@ -933,14 +919,6 @@ operator_lt::fold_range (irange &r, tree type, const frange &op1, const frange &op2, relation_trio trio) const { - relation_kind rel = trio.op1_op2 (); - - // VREL_EQ & LT_EXPR is impossible, even with NANs. - if (rel == VREL_EQ) - { - r = range_false (type); - return true; - } if (frelop_early_resolve (r, type, op1, op2, trio, VREL_LT)) return true; @@ -1162,14 +1140,6 @@ operator_gt::fold_range (irange &r, tree type, const frange &op1, const frange &op2, relation_trio trio) const { - relation_kind rel = trio.op1_op2 (); - - // VREL_EQ & GT_EXPR is impossible, even with NANs. - if (rel == VREL_EQ) - { - r = range_false (type); - return true; - } if (frelop_early_resolve (r, type, op1, op2, trio, VREL_GT)) return true; @@ -2129,10 +2099,7 @@ public: const frange &op1, const frange &op2, relation_trio trio = TRIO_VARYING) const final override { - relation_kind rel = trio.op1_op2 (); - - if (op1.known_isnan () || op2.known_isnan () - || rel == VREL_EQ) + if (op1.known_isnan () || op2.known_isnan ()) { r = range_true (type); return true; diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-12.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-1.c similarity index 100% rename from gcc/testsuite/gcc.dg/tree-ssa/vrp-float-12.c rename to gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-1.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-2.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-2.c new file mode 100644 index 00000000000..76c91302167 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-2.c @@ -0,0 +1,21 @@ +// { dg-do compile } +// { dg-options "-O2 -fdump-tree-threadfull1-details" } + +void stuff(); + +void foo (float a, int cond) +{ + float x; + + if (cond) + x = a; + else + x = 8.0; + + /* We should be able to fold this as false on the path coming out of + cond == TRUE conditional. */ + if (x < a) + stuff(); +} + +// { dg-final { scan-tree-dump "Registering jump thread: \\(2, 4\\)" "threadfull1" } } diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-3.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-3.c new file mode 100644 index 00000000000..b7389f5ce26 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-3.c @@ -0,0 +1,27 @@ +// { dg-do compile } +// { dg-options "-O2 -fdump-tree-all-stats" } + +void stuff(); + +void foo (float a, int cond) +{ + float x; + + if (cond) + x = a; + else + x = 8.0; + + /* Even though on the BB2->BB4 path, x_1 and a_4 are equivalent, we cannot + fold x_1 == a_4 because of possible NANs: + + + # x_1 = PHI + if (x_1 == a_4(D)) + */ + if (x == a) + stuff(); +} + +// { dg-final { scan-tree-dump-not "Jumps threaded" "threadfull1" } } +// { dg-final { scan-tree-dump-not "Jumps threaded" "threadfull2" } } diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-4.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-4.c new file mode 100644 index 00000000000..f38b305f4af --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-relations-4.c @@ -0,0 +1,38 @@ +// { dg-do compile } +// { dg-options "-O2 -fgimple -fdump-tree-threadfull1-details" } + +void stuff(); + +void __GIMPLE (ssa,startwith("threadfull1")) +foo (float a, float b, int cond) +{ + float x; + + __BB(2): + if (cond_3(D) != 0) + goto __BB4; + else + goto __BB3; + + __BB(3): + goto __BB4; + + /* We should be able to thread BB2->BB4->BB5 even though we have no knowledge + of the NANness of either x_1 or a_5. */ + __BB(4): + x_1 = __PHI (__BB2: a_5(D), __BB3: b_4(D)); + if (x_1 __UNEQ a_5(D)) + goto __BB5; + else + goto __BB6; + + __BB(5): + stuff (); + goto __BB6; + + __BB(6): + return; + +} + +// { dg-final { scan-tree-dump "Registering jump thread: \\(2, 4\\)" "threadfull1" } } -- 2.41.0