* [PATCH] PR tree-optimization/109274 - Don't interpret contents of a value_relation record.
@ 2023-03-24 15:08 Andrew MacLeod
2023-03-24 15:15 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Andrew MacLeod @ 2023-03-24 15:08 UTC (permalink / raw)
To: gcc-patches, Jakub Jelinek, Richard Biener
[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]
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
[-- Attachment #2: 274.diff --]
[-- Type: text/x-patch, Size: 2172 bytes --]
commit 1f02961b23976d35b10e2399708c6eb00632f9d6
Author: Andrew MacLeod <amacleod@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
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.
The bug occurs because the "sanitized" records doesnt set op1 anmd op2,
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.
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/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;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] PR tree-optimization/109274 - Don't interpret contents of a value_relation record. 2023-03-24 15:08 [PATCH] PR tree-optimization/109274 - Don't interpret contents of a value_relation record Andrew MacLeod @ 2023-03-24 15:15 ` Jakub Jelinek 2023-03-24 15:52 ` Andrew MacLeod 0 siblings, 1 reply; 6+ messages in thread From: Jakub Jelinek @ 2023-03-24 15:15 UTC (permalink / raw) To: Andrew MacLeod; +Cc: gcc-patches, Richard Biener 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 <amacleod@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PR tree-optimization/109274 - Don't interpret contents of a value_relation record. 2023-03-24 15:15 ` Jakub Jelinek @ 2023-03-24 15:52 ` Andrew MacLeod 2023-03-24 16:36 ` Jakub Jelinek 0 siblings, 1 reply; 6+ messages in thread From: Andrew MacLeod @ 2023-03-24 15:52 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches, Richard Biener [-- Attachment #1: Type: text/plain, Size: 3006 bytes --] 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 <amacleod@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 > [-- Attachment #2: 274b.diff --] [-- Type: text/x-patch, Size: 3321 bytes --] commit 2d8ef296f43c930a1822817e3280539ce5e7075b Author: Andrew MacLeod <amacleod@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; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PR tree-optimization/109274 - Don't interpret contents of a value_relation record. 2023-03-24 15:52 ` Andrew MacLeod @ 2023-03-24 16:36 ` Jakub Jelinek 2023-03-28 13:19 ` [PATCH] PR tree-optimization/109274 -Fix compute_operand when op1 == op2 symbolically Andrew MacLeod 0 siblings, 1 reply; 6+ messages in thread From: Jakub Jelinek @ 2023-03-24 16:36 UTC (permalink / raw) To: Andrew MacLeod, Aldy Hernandez; +Cc: gcc-patches, Richard Biener On Fri, Mar 24, 2023 at 11:52:30AM -0400, Andrew MacLeod wrote: > Thanks.. Ive incorporated it into my commit too. Note, both my earlier version of the patch and your patch regress: FAIL: gcc.dg/tree-ssa/vrp-float-3a.c scan-tree-dump-not evrp "link_error" FAIL: gcc.dg/tree-ssa/vrp-float-4a.c scan-tree-dump-not evrp "link_error" Jakub ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] PR tree-optimization/109274 -Fix compute_operand when op1 == op2 symbolically. 2023-03-24 16:36 ` Jakub Jelinek @ 2023-03-28 13:19 ` Andrew MacLeod 2023-03-28 13:28 ` Richard Biener 0 siblings, 1 reply; 6+ messages in thread From: Andrew MacLeod @ 2023-03-28 13:19 UTC (permalink / raw) To: Jakub Jelinek, Aldy Hernandez; +Cc: gcc-patches, Richard Biener [-- Attachment #1: Type: text/plain, Size: 2222 bytes --] On 3/24/23 12:36, Jakub Jelinek wrote: > On Fri, Mar 24, 2023 at 11:52:30AM -0400, Andrew MacLeod wrote: >> Thanks.. Ive incorporated it into my commit too. > Note, both my earlier version of the patch and your patch regress: > FAIL: gcc.dg/tree-ssa/vrp-float-3a.c scan-tree-dump-not evrp "link_error" > FAIL: gcc.dg/tree-ssa/vrp-float-4a.c scan-tree-dump-not evrp "link_error" > > Jakub > OK, that was fun. I commented in the PR, but the root issue is the way I was trying to communicate symbolic equivalence on operands to the compute_operand routines. [1, 1 ]= a_1 != a_1 I was creating a relation record between op1 and op2 indicating they were equivalent. THis record then gets passed up the gori call chain. Reality is that this is not a true equivalence... without looking a the ranges, we dont know that that is true for general application, and and furthermore, when applied to something like a1 != a1, you can see the problem... Once I corrected the value_relation record to create records with the same operand, things went south. What we really need is to locally identify when op1 and op2 are the same symbol, and if there is no other information, pass it locally on that one statement to the range-op handler. Then, once we have processed the statement, we invoke the handler for that statement to cvreate a record which is passed up the chain. so for: a_1 = b_4 + 1.0 [1, 1] = (a_1 != a_1) compute_operand_1 starts with no relation record, recognizes symbolically op1 and op2 are the same, and passes EQ_EXPR locally as the op1_op2 relation to the handler for NE_EXPR. That handler utilizes the range of op2 to detemine whether != is true or not based on knowledge that op1 and op2 are the same value. (for integer always false, for float, takes a look at NAN) and produces a result. Before invoking compute_operand to calculate b_4 with the result of a_1, handler.op1_op2_relation (lhs); is invoked to determine if there is a relation generated by the statement, which will generate (a_1 != a_1), and pass that to compute operand for use in evaluating b_4. Bootstraps on x86_64-pc-linux-gnu with no regressions. OK for trunk? Andrew [-- Attachment #2: 274f.diff --] [-- Type: text/x-patch, Size: 7269 bytes --] commit 97f9c24fe471b9701cf3ba89901e146a9c52f3ad Author: Andrew MacLeod <amacleod@redhat.com> Date: Fri Mar 24 11:21:20 2023 -0400 Fix compute_operand when op1 == op2 symbolically. First, class value_relation should not sanitize records. just create what is asked. Second., if there is not a relation record, compute_operand was creating one for op1 == op2 if op1 and op2 were the same symbol. This is not the correct way to communicate the information, as that record will continue to be passed along the GORI unwind chain. Instead, simply pass that information locally to the opX_range routine for only the current statement. PR tree-optimization/109265 PR tree-optimization/109274 gcc/ * gimple-range-gori.cc (gori_compute::compute_operand_range): Do not create a relation record is op1 and op2 are the same symbol. (gori_compute::compute_operand1_range): Pass op1 == op2 to the handler for this stmt, but create a new record only if this statement generates a relation based on the ranges. (gori_compute::compute_operand2_range): Ditto. * 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/gimple-range-gori.cc b/gcc/gimple-range-gori.cc index 3c0044881fb..b9c3ba1a314 100644 --- a/gcc/gimple-range-gori.cc +++ b/gcc/gimple-range-gori.cc @@ -623,21 +623,6 @@ gori_compute::compute_operand_range (vrange &r, gimple *stmt, tree op1 = gimple_range_ssa_p (handler.operand1 ()); tree op2 = gimple_range_ssa_p (handler.operand2 ()); - // If there is a relation, use it instead of any passed in. This will allow - // multiple relations to be processed in compound logicals. - if (op1 && op2) - { - relation_kind k = handler.op1_op2_relation (lhs); - // If there is no relation, and op1 == op2, create a relation. - if (!vrel_ptr && k == VREL_VARYING && op1 == op2) - k = VREL_EQ; - if (k != VREL_VARYING) - { - vrel.set_relation (k, op1, op2); - vrel_ptr = &vrel; - } - } - // Handle end of lookup first. if (op1 == name) return compute_operand1_range (r, handler, lhs, name, src, vrel_ptr); @@ -1093,6 +1078,7 @@ gori_compute::compute_operand1_range (vrange &r, const vrange &lhs, tree name, fur_source &src, value_relation *rel) { + value_relation local_rel; gimple *stmt = handler.stmt (); tree op1 = handler.operand1 (); tree op2 = handler.operand2 (); @@ -1101,6 +1087,7 @@ gori_compute::compute_operand1_range (vrange &r, relation_trio trio; if (rel) trio = rel->create_trio (lhs_name, op1, op2); + relation_kind op_op = trio.op1_op2 (); Value_Range op1_range (TREE_TYPE (op1)); Value_Range tmp (TREE_TYPE (op1)); @@ -1113,10 +1100,26 @@ gori_compute::compute_operand1_range (vrange &r, if (op2) { src.get_operand (op2_range, op2); - relation_kind op_op = trio.op1_op2 (); + + // If there is a relation betwen op1 and op2, use it instead. + // This allows multiple relations to be processed in compound logicals. + if (gimple_range_ssa_p (op1) && gimple_range_ssa_p (op2)) + { + relation_kind k = handler.op1_op2_relation (lhs); + if (k != VREL_VARYING) + { + op_op = k; + local_rel.set_relation (op_op, op1, op2); + rel = &local_rel; + } + } + if (op_op != VREL_VARYING) refine_using_relation (op1, op1_range, op2, op2_range, src, op_op); + // If op1 == op2, create a new trio for just this call. + if (op1 == op2 && gimple_range_ssa_p (op1)) + trio = relation_trio (trio.lhs_op1 (), trio.lhs_op2 (), VREL_EQ); if (!handler.calc_op1 (tmp, lhs, op2_range, trio)) return false; } @@ -1185,6 +1188,7 @@ gori_compute::compute_operand2_range (vrange &r, const vrange &lhs, tree name, fur_source &src, value_relation *rel) { + value_relation local_rel; gimple *stmt = handler.stmt (); tree op1 = handler.operand1 (); tree op2 = handler.operand2 (); @@ -1201,9 +1205,26 @@ gori_compute::compute_operand2_range (vrange &r, if (rel) trio = rel->create_trio (lhs_name, op1, op2); relation_kind op_op = trio.op1_op2 (); + + // If there is a relation betwen op1 and op2, use it instead. + // This allows multiple relations to be processed in compound logicals. + if (gimple_range_ssa_p (op1) && gimple_range_ssa_p (op2)) + { + relation_kind k = handler.op1_op2_relation (lhs); + if (k != VREL_VARYING) + { + op_op = k; + local_rel.set_relation (op_op, op1, op2); + rel = &local_rel; + } + } + if (op_op != VREL_VARYING) refine_using_relation (op1, op1_range, op2, op2_range, src, op_op); + // If op1 == op2, create a new trio for this stmt. + if (op1 == op2 && gimple_range_ssa_p (op1)) + trio = relation_trio (trio.lhs_op1 (), trio.lhs_op2 (), VREL_EQ); // Intersect with range for op2 based on lhs and op1. if (!handler.calc_op2 (tmp, lhs, op1_range, trio)) return false; 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; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PR tree-optimization/109274 -Fix compute_operand when op1 == op2 symbolically. 2023-03-28 13:19 ` [PATCH] PR tree-optimization/109274 -Fix compute_operand when op1 == op2 symbolically Andrew MacLeod @ 2023-03-28 13:28 ` Richard Biener 0 siblings, 0 replies; 6+ messages in thread From: Richard Biener @ 2023-03-28 13:28 UTC (permalink / raw) To: Andrew MacLeod; +Cc: Jakub Jelinek, Aldy Hernandez, gcc-patches On Tue, Mar 28, 2023 at 3:19 PM Andrew MacLeod <amacleod@redhat.com> wrote: > > On 3/24/23 12:36, Jakub Jelinek wrote: > > On Fri, Mar 24, 2023 at 11:52:30AM -0400, Andrew MacLeod wrote: > >> Thanks.. Ive incorporated it into my commit too. > > Note, both my earlier version of the patch and your patch regress: > > FAIL: gcc.dg/tree-ssa/vrp-float-3a.c scan-tree-dump-not evrp "link_error" > > FAIL: gcc.dg/tree-ssa/vrp-float-4a.c scan-tree-dump-not evrp "link_error" > > > > Jakub > > > OK, that was fun. > > I commented in the PR, but the root issue is the way I was trying to > communicate symbolic equivalence on operands to the compute_operand > routines. > > [1, 1 ]= a_1 != a_1 > > I was creating a relation record between op1 and op2 indicating they > were equivalent. THis record then gets passed up the gori call chain. > > Reality is that this is not a true equivalence... without looking a the > ranges, we dont know that that is true for general application, and and > furthermore, when applied to something like a1 != a1, you can see the > problem... > > Once I corrected the value_relation record to create records with the > same operand, things went south. > > What we really need is to locally identify when op1 and op2 are the same > symbol, and if there is no other information, pass it locally on that > one statement to the range-op handler. > > Then, once we have processed the statement, we invoke the handler for > that statement to cvreate a record which is passed up the chain. > so for: > > a_1 = b_4 + 1.0 > [1, 1] = (a_1 != a_1) > > compute_operand_1 starts with no relation record, recognizes > symbolically op1 and op2 are the same, and passes EQ_EXPR locally as the > op1_op2 relation to the handler for NE_EXPR. That handler utilizes the > range of op2 to detemine whether != is true or not based on knowledge > that op1 and op2 are the same value. (for integer always false, for > float, takes a look at NAN) and produces a result. > > Before invoking compute_operand to calculate b_4 with the result of a_1, > handler.op1_op2_relation (lhs); > is invoked to determine if there is a relation generated by the > statement, which will generate (a_1 != a_1), and pass that to compute > operand for use in evaluating b_4. > > Bootstraps on x86_64-pc-linux-gnu with no regressions. OK for trunk? LGTM. Thanks, Richard. > Andrew ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-28 13:29 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-24 15:08 [PATCH] PR tree-optimization/109274 - Don't interpret contents of a value_relation record Andrew MacLeod 2023-03-24 15:15 ` Jakub Jelinek 2023-03-24 15:52 ` Andrew MacLeod 2023-03-24 16:36 ` Jakub Jelinek 2023-03-28 13:19 ` [PATCH] PR tree-optimization/109274 -Fix compute_operand when op1 == op2 symbolically Andrew MacLeod 2023-03-28 13:28 ` Richard Biener
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).