This patch adds VREL_OTHER to represent another relation we do not
understand. It is used to represent the class fo relations arising from
floating point that are currently not represented. IN GCC 14 we will
determine exactly how to represent them, but for this release, this
should prevent us from getting things wrong at least.
if intersection produces UNDEFINED and either of the relations feeding
it were <, <=, >, or >= then it turns it to VR_OTHER. the prevents
false sides of branches from combining to produce UNDEFINED when they
end up being a known NAN.
Union is adjusted such that < >, or <= >= also produce VREL_OTHER. < >
cannot be properly represented, and <= >= was producing VARYING, which
is also not correct.
Form the testcase:
<bb 2> :
cmp1_10 = x_8(D) <= y_9(D);
cmp2_11 = x_8(D) >= y_9(D);
_3 = cmp1_10 & cmp2_11;
if (_3 != 0)
goto <bb 3>; [INV]
else
goto <bb 4>; [INV]
Relational : (x_8(D) == y_9(D))
<bb 3> :
// predicted unlikely by early return (on trees) predictor.
goto <bb 6>; [INV]
<bb 4> :
_4 = ~cmp1_10;
_5 = ~cmp2_11;
_1 = cmp1_10 | cmp2_11;
_6 = ~_1;
if (_1 != 0)
goto <bb 6>; [INV]
else
goto <bb 5>; [INV]
Relational : (x_8(D) unknown fp y_9(D))
<bb 5> :
// predicted unlikely by early return (on trees) predictor.
The intersection "fix" is represented by the relation between x and y in
BB5. Without the patch, ti would be UNDEFINED and we do not want that.
The union fix is what prevents us from folding the condition in bb_4.
Without it, x<=y union x >=y comes out VARYING, when in fact I believe
it represents everything except a NAN.
So with this fix, all the unrepresentative relations get lumped into
VREL_OTHER so we at least don't get it wrong. For next release we can
take the time to figure out exactly how we want to proceed.
This is currently in testing on x86_64-pc-linux-gnu, and assuming it
bootstraps with no regressions (a combined patch did before splitting it
in 2), OK for trunk? OR does it need more tweaking?
Andrew
From 6947cfa03098eee46669ec2902e5f6e33c3cbe9a Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacl...@redhat.com>
Date: Fri, 20 Jan 2023 17:31:26 -0500
Subject: [PATCH 2/2] Add VREL_OTHER for FP unsupported relations.
Add VREL_OTHER to represent floating point relations we do not yet
support.
PR tree-optimization/108447
gcc/
* value-relation.cc (kind_string): Add "unknown fp".
(rr_negate_table): Add entry for VREL_OTHER.
(rr_swap_table): Likewise.
(rr_intersect_table): Likewise.
(rr_union_table): Likewise.
(rr_transitive_table): Likewise.
(relation_to_code): Likewise.
(value_relation::intersect): Handle floating point differences.
(value_relation::union_): Likewise.
* value-relation.h (enum relation_kind_t): Add VREL_OTHER.
gcc/testsuite/
* gcc.dg/pr108447.c: New.
---
gcc/testsuite/gcc.dg/pr108447.c | 33 +++++++++
gcc/value-relation.cc | 118 +++++++++++++++++++++-----------
gcc/value-relation.h | 1 +
3 files changed, 113 insertions(+), 39 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr108447.c
diff --git a/gcc/testsuite/gcc.dg/pr108447.c b/gcc/testsuite/gcc.dg/pr108447.c
new file mode 100644
index 00000000000..cfbaba6d0aa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr108447.c
@@ -0,0 +1,33 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+__attribute__((noipa)) int
+foo (float x, float y)
+{
+ _Bool cmp1 = x <= y;
+ _Bool cmp2 = x >= y;
+ if (cmp1 && cmp2)
+ return 1;
+ else if (!cmp1 && !cmp2)
+ return -1;
+ return 0;
+}
+
+int
+main ()
+{
+ if (foo (0.0f, __builtin_nanf ("")) != -1)
+ __builtin_abort ();
+ if (foo (__builtin_nanf (""), -42.0f) != -1)
+ __builtin_abort ();
+ if (foo (0.0f, -0.0f) != 1)
+ __builtin_abort ();
+ if (foo (42.0f, 42.0f) != 1)
+ __builtin_abort ();
+ if (foo (42.0f, -0.0f) != 0)
+ __builtin_abort ();
+ if (foo (0.0f, -42.0f) != 0)
+ __builtin_abort ();
+ return 0;
+}
+
diff --git a/gcc/value-relation.cc b/gcc/value-relation.cc
index 432828e2b13..299a119827a 100644
--- a/gcc/value-relation.cc
+++ b/gcc/value-relation.cc
@@ -33,8 +33,8 @@ along with GCC; see the file COPYING3. If not see
#include "dominance.h"
static const char *kind_string[VREL_LAST] =
-{ "varying", "undefined", "<", "<=", ">", ">=", "==", "!=", "pe8", "pe16",
- "pe32", "pe64" };
+{ "varying", "undefined", "<", "<=", ">", ">=", "==", "!=", "unknown fp",
+ "pe8", "pe16", "pe32", "pe64" };
// Print a relation_kind REL to file F.
@@ -47,7 +47,7 @@ print_relation (FILE *f, relation_kind rel)
// This table is used to negate the operands. op1 REL op2 -> !(op1 REL op2).
relation_kind rr_negate_table[VREL_LAST] = {
VREL_VARYING, VREL_UNDEFINED, VREL_GE, VREL_GT, VREL_LE, VREL_LT, VREL_NE,
- VREL_EQ };
+ VREL_EQ, VREL_OTHER };
// Negate the relation, as in logical negation.
@@ -60,7 +60,7 @@ relation_negate (relation_kind r)
// This table is used to swap the operands. op1 REL op2 -> op2 REL op1.
relation_kind rr_swap_table[VREL_LAST] = {
VREL_VARYING, VREL_UNDEFINED, VREL_GT, VREL_GE, VREL_LT, VREL_LE, VREL_EQ,
- VREL_NE };
+ VREL_NE, VREL_OTHER };
// Return the relation as if the operands were swapped.
@@ -75,28 +75,32 @@ relation_swap (relation_kind r)
relation_kind rr_intersect_table[VREL_LAST][VREL_LAST] = {
// VREL_VARYING
{ VREL_VARYING, VREL_UNDEFINED, VREL_LT, VREL_LE, VREL_GT, VREL_GE, VREL_EQ,
- VREL_NE },
+ VREL_NE, VREL_OTHER },
// VREL_UNDEFINED
{ VREL_UNDEFINED, VREL_UNDEFINED, VREL_UNDEFINED, VREL_UNDEFINED,
- VREL_UNDEFINED, VREL_UNDEFINED, VREL_UNDEFINED, VREL_UNDEFINED },
+ VREL_UNDEFINED, VREL_UNDEFINED, VREL_UNDEFINED, VREL_UNDEFINED,
+ VREL_UNDEFINED },
// VREL_LT
{ VREL_LT, VREL_UNDEFINED, VREL_LT, VREL_LT, VREL_UNDEFINED, VREL_UNDEFINED,
- VREL_UNDEFINED, VREL_LT },
+ VREL_UNDEFINED, VREL_LT, VREL_OTHER },
// VREL_LE
{ VREL_LE, VREL_UNDEFINED, VREL_LT, VREL_LE, VREL_UNDEFINED, VREL_EQ,
- VREL_EQ, VREL_LT },
+ VREL_EQ, VREL_LT, VREL_OTHER },
// VREL_GT
{ VREL_GT, VREL_UNDEFINED, VREL_UNDEFINED, VREL_UNDEFINED, VREL_GT, VREL_GT,
- VREL_UNDEFINED, VREL_GT },
+ VREL_UNDEFINED, VREL_GT, VREL_OTHER },
// VREL_GE
{ VREL_GE, VREL_UNDEFINED, VREL_UNDEFINED, VREL_EQ, VREL_GT, VREL_GE,
- VREL_EQ, VREL_GT },
+ VREL_EQ, VREL_GT, VREL_OTHER },
// VREL_EQ
{ VREL_EQ, VREL_UNDEFINED, VREL_UNDEFINED, VREL_EQ, VREL_UNDEFINED, VREL_EQ,
- VREL_EQ, VREL_UNDEFINED },
+ VREL_EQ, VREL_UNDEFINED, VREL_OTHER },
// VREL_NE
{ VREL_NE, VREL_UNDEFINED, VREL_LT, VREL_LT, VREL_GT, VREL_GT,
- VREL_UNDEFINED, VREL_NE } };
+ VREL_UNDEFINED, VREL_NE, VREL_OTHER },
+// VREL_OTHER
+ { VREL_OTHER, VREL_UNDEFINED, VREL_OTHER, VREL_OTHER, VREL_OTHER,
+ VREL_OTHER, VREL_OTHER, VREL_OTHER, VREL_OTHER } };
// Intersect relation R1 with relation R2 and return the resulting relation.
@@ -113,28 +117,31 @@ relation_intersect (relation_kind r1, relation_kind r2)
relation_kind rr_union_table[VREL_LAST][VREL_LAST] = {
// VREL_VARYING
{ VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING,
- VREL_VARYING, VREL_VARYING, VREL_VARYING },
+ VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING },
// VREL_UNDEFINED
{ VREL_VARYING, VREL_UNDEFINED, VREL_LT, VREL_LE, VREL_GT, VREL_GE,
- VREL_EQ, VREL_NE },
+ VREL_EQ, VREL_NE, VREL_OTHER },
// VREL_LT
{ VREL_VARYING, VREL_LT, VREL_LT, VREL_LE, VREL_NE, VREL_VARYING, VREL_LE,
- VREL_NE },
+ VREL_NE, VREL_OTHER },
// VREL_LE
{ VREL_VARYING, VREL_LE, VREL_LE, VREL_LE, VREL_VARYING, VREL_VARYING,
- VREL_LE, VREL_VARYING },
+ VREL_LE, VREL_VARYING, VREL_OTHER },
// VREL_GT
{ VREL_VARYING, VREL_GT, VREL_NE, VREL_VARYING, VREL_GT, VREL_GE, VREL_GE,
- VREL_NE },
+ VREL_NE, VREL_OTHER },
// VREL_GE
{ VREL_VARYING, VREL_GE, VREL_VARYING, VREL_VARYING, VREL_GE, VREL_GE,
- VREL_GE, VREL_VARYING },
+ VREL_GE, VREL_VARYING, VREL_OTHER },
// VREL_EQ
{ VREL_VARYING, VREL_EQ, VREL_LE, VREL_LE, VREL_GE, VREL_GE, VREL_EQ,
- VREL_VARYING },
+ VREL_VARYING, VREL_OTHER },
// VREL_NE
{ VREL_VARYING, VREL_NE, VREL_NE, VREL_VARYING, VREL_NE, VREL_VARYING,
- VREL_VARYING, VREL_NE } };
+ VREL_VARYING, VREL_NE, VREL_OTHER },
+// VREL_OTHER
+ { VREL_VARYING, VREL_OTHER, VREL_OTHER, VREL_OTHER, VREL_OTHER,
+ VREL_OTHER, VREL_OTHER, VREL_OTHER, VREL_OTHER } };
// Union relation R1 with relation R2 and return the result.
@@ -151,28 +158,31 @@ relation_union (relation_kind r1, relation_kind r2)
relation_kind rr_transitive_table[VREL_LAST][VREL_LAST] = {
// VREL_VARYING
{ VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING,
- VREL_VARYING, VREL_VARYING, VREL_VARYING },
+ VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING },
// VREL_UNDEFINED
{ VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING,
- VREL_VARYING, VREL_VARYING, VREL_VARYING },
+ VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING },
// VREL_LT
{ VREL_VARYING, VREL_VARYING, VREL_LT, VREL_LT, VREL_VARYING, VREL_VARYING,
- VREL_LT, VREL_VARYING },
+ VREL_LT, VREL_VARYING, VREL_VARYING },
// VREL_LE
{ VREL_VARYING, VREL_VARYING, VREL_LT, VREL_LE, VREL_VARYING, VREL_VARYING,
- VREL_LE, VREL_VARYING },
+ VREL_LE, VREL_VARYING, VREL_VARYING },
// VREL_GT
{ VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_GT, VREL_GT,
- VREL_GT, VREL_VARYING },
+ VREL_GT, VREL_VARYING, VREL_VARYING },
// VREL_GE
{ VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_GT, VREL_GE,
- VREL_GE, VREL_VARYING },
+ VREL_GE, VREL_VARYING, VREL_VARYING },
// VREL_EQ
{ VREL_VARYING, VREL_VARYING, VREL_LT, VREL_LE, VREL_GT, VREL_GE, VREL_EQ,
- VREL_VARYING },
+ VREL_VARYING, VREL_VARYING },
// VREL_NE
{ VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING,
- VREL_VARYING, VREL_VARYING, VREL_VARYING } };
+ VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING },
+// VREL_OTHER
+ { VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING,
+ VREL_VARYING, VREL_VARYING, VREL_VARYING, VREL_VARYING } };
// Apply transitive operation between relation R1 and relation R2, and
// return the resulting relation, if any.
@@ -187,7 +197,7 @@ relation_transitive (relation_kind r1, relation_kind r2)
tree_code relation_to_code [VREL_LAST] = {
ERROR_MARK, ERROR_MARK, LT_EXPR, LE_EXPR, GT_EXPR, GE_EXPR, EQ_EXPR,
- NE_EXPR };
+ NE_EXPR, ERROR_MARK };
// This routine validates that a relation can be applied to a specific set of
// ranges. In particular, floating point x == x may not be true if the NaN bit
@@ -784,17 +794,28 @@ value_relation::negate ()
bool
value_relation::intersect (const value_relation &p)
{
- // Save previous value
- relation_kind old = related;
+ relation_kind k;
if (p.op1 () == op1 () && p.op2 () == op2 ())
- related = relation_intersect (kind (), p.kind ());
+ k = relation_intersect (kind (), p.kind ());
else if (p.op2 () == op1 () && p.op1 () == op2 ())
- related = relation_intersect (kind (), relation_swap (p.kind ()));
+ k = relation_intersect (kind (), relation_swap (p.kind ()));
else
return false;
- return old != related;
+ if (related == k)
+ return false;
+
+ bool float_p = name1 && FLOAT_TYPE_P (TREE_TYPE (name1));
+ if (float_p && p.kind () != k && k == VREL_UNDEFINED)
+ {
+ if (relation_lt_le_gt_ge_p (kind ())
+ || relation_lt_le_gt_ge_p (p.kind ()))
+ k = VREL_OTHER;
+ }
+
+ related = k;
+ return true;
}
// Perform a union between 2 relations. *this ||= p.
@@ -802,17 +823,36 @@ value_relation::intersect (const value_relation &p)
bool
value_relation::union_ (const value_relation &p)
{
- // Save previous value
- relation_kind old = related;
+ relation_kind k;
if (p.op1 () == op1 () && p.op2 () == op2 ())
- related = relation_union (kind(), p.kind());
+ k = relation_union (kind (), p.kind ());
else if (p.op2 () == op1 () && p.op1 () == op2 ())
- related = relation_union (kind(), relation_swap (p.kind ()));
+ k = relation_union (kind (), relation_swap (p.kind ()));
else
return false;
- return old != related;
+ if (related == k)
+ return false;
+
+ // (x < y) || (x > y) produces x != y, but this is not true with floats.
+ // (x <= y) || (x >= y) produces VARYING, which is also not true for floats.
+ // As they cannot be properly represented, use VREL_OTHER.
+ bool float_p = name1 && FLOAT_TYPE_P (TREE_TYPE (name1));
+ if (float_p && p.kind () != k)
+ {
+ if (kind () == VREL_LT && p.kind () == VREL_GT)
+ k = VREL_OTHER;
+ else if (kind () == VREL_GT && p.kind () == VREL_LT)
+ k = VREL_OTHER;
+ else if (kind () == VREL_LE && p.kind () == VREL_GE)
+ k = VREL_OTHER;
+ else if (kind () == VREL_GE && p.kind () == VREL_LE)
+ k = VREL_OTHER;
+ }
+
+ related = k;
+ return true;
}
// Identify and apply any transitive relations between REL
diff --git a/gcc/value-relation.h b/gcc/value-relation.h
index 354a0fd4130..c191de292c7 100644
--- a/gcc/value-relation.h
+++ b/gcc/value-relation.h
@@ -70,6 +70,7 @@ typedef enum relation_kind_t
VREL_GE, // r1 >= r2
VREL_EQ, // r1 == r2
VREL_NE, // r1 != r2
+ VREL_OTHER, // unrepresentatble floating point relation.
VREL_PE8, // 8 bit partial equivalency
VREL_PE16, // 16 bit partial equivalency
VREL_PE32, // 32 bit partial equivalency
--
2.39.0
