Thanks.. Ive incorporated it into my commit too.
Andrew
On 3/24/23 11:15, Jakub Jelinek wrote:
On Fri, Mar 24, 2023 at 11:08:54AM -0400, Andrew MacLeod wrote:
Before floating point relations were added, we tried to sanitize
value-relation records to not include non-sensensical records... ie x != x
or x < x. Instead, we made a VREL_VARYING record with no operands.
When floating point relation support was added, some of these were no longer
non-sensical, AND we expanded the use of value_relation records into GORI
shortly thereafter.
As a result, this sanitization is no longer needed, nor desired. The Oracle
does not create records with op1 == op2 already, so its only within GORI
that these records can exist, and we shouldn't try to interpret them.
The bug occurs because the "sanitized" records doesn't set op1 and op2, and
changes the relation to VARYING.. and we expected the operands it to be set
the way they were specified. We should not be setting a VREL_VARYING record
if asked to set something else. In fact, we are missing some opportunities
because we are trying to FP range-ops that op1 != op1 but its getting
transformed into a VREL_VARYING record and not communicated properly.
Currently bootstrapping on x86_64-pc-linux-gnu and assuming no regressions,
OK for trunk?
Andrew
commit 1f02961b23976d35b10e2399708c6eb00632f9d6
Author: Andrew MacLeod <amacl...@redhat.com>
Date: Fri Mar 24 09:18:33 2023 -0400
Don't interpret contents of a value_relation record.
before floating point relations were added, we tried to sanitize
value-relation records to not include non-sensensical records... ie
x != x or x < x. INstead, we made a VREL_VARYING record with no
s/IN/In/
operands.
When floating point relations were supported, some of these were no
longer non-sensical, AND we expanded the use of value_relation records
into GORI.
As a result, this sanitization is no longer needed. The Oracle
does not create records with op1 == op2, so its only within GORI
that these records can exist, and we shouldnt try to interpret them.
s/shouldnt/shouldn't/
The bug occurs because the "sanitized" records doesnt set op1 anmd op2,
s/doesnt/doesn't/
but we have a record so expected it to be set.
PR tree-optimization/109265
PR tree-optimization/109274
gcc/
* value-relation.h (value_relation::set_relation): Always create
the
record that is requested.
gcc/testsuite/
* gcc.dg/pr109274.c: New.
LGTM, indeed with floating point a != a isn't nonsensical but basically
__builtin_isnan (a) check.
I'll commit the Fortran testcase I've added in my version of the patch
incrementally when you commit.
Jakub
commit 2d8ef296f43c930a1822817e3280539ce5e7075b
Author: Andrew MacLeod <amacl...@redhat.com>
Date: Fri Mar 24 11:21:20 2023 -0400
Don't interpret contents of a value_relation record.
Before floating point relations were added, we tried to sanitize
value-relation records to not include non-sensensical records... ie
x != x or x < x. Instead, we made a VREL_VARYING record with no
operands.
When floating point relations were supported, some of these were no
longer non-sensical, AND we shortly expanded the use of value_relation
records into GORI.
As a result, this sanitization is no longer needed. The Oracle
does not create records with op1 == op2, so its only within GORI
that these records can exist, and we shouldn't try to interpret them.
The bug occurs because the "sanitized" records doesn't set op1 anmd op2,
we should simply set the record with the specified operands..
PR tree-optimization/109265
PR tree-optimization/109274
gcc/
* value-relation.h (value_relation::set_relation): Always create the
record that is requested.
gcc/testsuite/
* gcc.dg/pr109274.c: New.
* gfortran.dg/pr109265.f90: New.
diff --git a/gcc/testsuite/gcc.dg/pr109274.c b/gcc/testsuite/gcc.dg/pr109274.c
new file mode 100644
index 00000000000..5dbc0232f8e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr109274.c
@@ -0,0 +1,16 @@
+/* PR tree-optimization/109274 */
+/* { dg-do compile } */
+/* { dg-options "-O2 " } */
+
+float a, b, c;
+int d;
+float bar (void);
+
+void
+foo (void)
+{
+ a = 0 * -(2.0f * c);
+ d = a != a ? 0 : bar ();
+ b = c;
+}
+
diff --git a/gcc/testsuite/gfortran.dg/pr109265.f90 b/gcc/testsuite/gfortran.dg/pr109265.f90
new file mode 100644
index 00000000000..0d7124c7741
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr109265.f90
@@ -0,0 +1,39 @@
+! PR tree-optimization/109265
+! { dg-do compile }
+! { dg-options "-O3 -w" }
+
+module pr109265
+ integer, parameter :: r8 = selected_real_kind (12)
+contains
+ subroutine foo (b, c, d, e, f)
+ implicit none
+ logical :: b
+ real (kind = r8) :: c, d, e, f, i
+ if (b) then
+ c = bar (c * d, e)
+ i = bar (f, c)
+ call baz (i)
+ call baz (-i)
+ end if
+ end subroutine foo
+ function bar (a, b)
+ implicit none
+ real (kind = r8) :: bar
+ real (kind = r8) :: a, b
+ bar = a + b
+ end function bar
+ subroutine baz (b)
+ implicit none
+ real (kind = r8) :: b, d, e, f, g, h, i
+ d = b
+ i = 0
+ e = d
+ f = d
+ g = d
+ 10 continue
+ if ((e.eq.d) .and. (f.eq.d) .and. (g.eq.d) .and. (h.eq.d)) then
+ h = i
+ goto 10
+ end if
+ end subroutine baz
+end module pr109265
diff --git a/gcc/value-relation.h b/gcc/value-relation.h
index 36a75862cc7..3177ecb1ad0 100644
--- a/gcc/value-relation.h
+++ b/gcc/value-relation.h
@@ -445,13 +445,6 @@ value_relation::set_relation (relation_kind r, tree n1, tree n2)
{
gcc_checking_assert (TREE_CODE (n1) == SSA_NAME
&& TREE_CODE (n2) == SSA_NAME);
- if (n1 == n2 && r != VREL_EQ)
- {
- related = VREL_VARYING;
- name1 = NULL_TREE;
- name2 = NULL_TREE;
- return;
- }
related = r;
name1 = n1;
name2 = n2;
