public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] PR tree-optimization/105458 - Check for equivalence after merging relations.
@ 2022-05-17 15:45 Andrew MacLeod
  2022-05-18  5:57 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Andrew MacLeod @ 2022-05-17 15:45 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 823 bytes --]

Sorry, missed this one earlier.

When we register a relation, such as LE_EXPR,  we first check if there 
is an existing relation that applies, and if so they are combined. We 
were checking if the relation being registered was an EQ_EXPR, and if 
so, invoked the equivalence oracle.

  I was doing the check for EQ_EXPR first, then merging with any 
existing relation.   In this case, the merge resulted in transforming 
the LE_EXPR into an EQ_EXPR, but the check to invoke the 
equivalence_oracle had already been done, and we got to a place we 
shouldn't have. doh!

The fix is to do the merge first, then check for EQ_EXPR.

The patch is a hair different (due to VREL_*  renames in gcc13), so I've 
attached both patches.

bootstraps on gcc12 and gcc13 with no regressions.  pushed on trunk.

OK for GCC12?

Andrew

[-- Attachment #2: pr105458.patch --]
[-- Type: text/x-patch, Size: 2014 bytes --]

commit 84c0e801f26275a4700c10a710a185c17f0418e5
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Mon May 16 21:39:30 2022 -0400

    Check for equivalence after merging relations.
    
    When registering a relation, we need to merge with any existing relation
    before checking if it was an equivalence... otherwise it was not being
    handled properly.
    
            gcc/
            PR tree-optimization/105458
            * value-relation.cc (path_oracle::register_relation): Merge, then check
            for equivalence.
    
            gcc/testsuite/
            * gcc.dg/pr105458.c: New.

diff --git a/gcc/testsuite/gcc.dg/pr105458.c b/gcc/testsuite/gcc.dg/pr105458.c
new file mode 100644
index 00000000000..eb58bf21f32
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr105458.c
@@ -0,0 +1,20 @@
+/* PR tree-optimization/105458 */
+/* { dg-do compile } */
+/* { dg-options "-O1 -fexpensive-optimizations -fno-tree-dominator-opts " } */
+
+void
+yj (int j4)
+{
+  int t3;
+
+  for (t3 = 0; t3 < 6; ++t3)
+    {
+      short int v4 = t3;
+
+      if (v4 == j4 || v4 > t3)
+        for (;;)
+          {
+          }
+    }
+}
+
diff --git a/gcc/value-relation.cc b/gcc/value-relation.cc
index a93565109f9..85d159f5d96 100644
--- a/gcc/value-relation.cc
+++ b/gcc/value-relation.cc
@@ -1384,16 +1384,16 @@ path_oracle::register_relation (basic_block bb, relation_kind k, tree ssa1,
       fprintf (dump_file, " (root: bb%d)\n", bb->index);
     }
 
+  relation_kind curr = query_relation (bb, ssa1, ssa2);
+  if (curr != VREL_VARYING)
+    k = relation_intersect (curr, k);
+
   if (k == VREL_EQ)
     {
       register_equiv (bb, ssa1, ssa2);
       return;
     }
 
-  relation_kind curr = query_relation (bb, ssa1, ssa2);
-  if (curr != VREL_VARYING)
-    k = relation_intersect (curr, k);
-
   bitmap_set_bit (m_relations.m_names, SSA_NAME_VERSION (ssa1));
   bitmap_set_bit (m_relations.m_names, SSA_NAME_VERSION (ssa2));
   relation_chain *ptr = (relation_chain *) obstack_alloc (&m_chain_obstack,

[-- Attachment #3: gcc12-pr105458.patch --]
[-- Type: text/x-patch, Size: 2008 bytes --]

commit c48fe8d3430e81ddea621c24e9b66d55aadfb316
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Tue May 17 09:36:39 2022 -0400

    Check for equivalence after merging relations.
    
    When registering a relation, we need to merge with any existing relation
    before checking if it was an equivalence... otherwise it was not being
    handled properly.
    
            gcc/
            PR tree-optimization/105458
            * value-relation.cc (path_oracle::register_relation): Merge, then check
            for equivalence.
    
            gcc/testsuite/
            * gcc.dg/pr105458.c: New.

diff --git a/gcc/testsuite/gcc.dg/pr105458.c b/gcc/testsuite/gcc.dg/pr105458.c
new file mode 100644
index 00000000000..eb58bf21f32
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr105458.c
@@ -0,0 +1,20 @@
+/* PR tree-optimization/105458 */
+/* { dg-do compile } */
+/* { dg-options "-O1 -fexpensive-optimizations -fno-tree-dominator-opts " } */
+
+void
+yj (int j4)
+{
+  int t3;
+
+  for (t3 = 0; t3 < 6; ++t3)
+    {
+      short int v4 = t3;
+
+      if (v4 == j4 || v4 > t3)
+        for (;;)
+          {
+          }
+    }
+}
+
diff --git a/gcc/value-relation.cc b/gcc/value-relation.cc
index 077ab4230a7..a69ad080e47 100644
--- a/gcc/value-relation.cc
+++ b/gcc/value-relation.cc
@@ -1388,16 +1388,16 @@ path_oracle::register_relation (basic_block bb, relation_kind k, tree ssa1,
       fprintf (dump_file, " (root: bb%d)\n", bb->index);
     }
 
+  relation_kind curr = query_relation (bb, ssa1, ssa2);
+  if (curr != VREL_NONE)
+    k = relation_intersect (curr, k);
+
   if (k == EQ_EXPR)
     {
       register_equiv (bb, ssa1, ssa2);
       return;
     }
 
-  relation_kind curr = query_relation (bb, ssa1, ssa2);
-  if (curr != VREL_NONE)
-    k = relation_intersect (curr, k);
-
   bitmap_set_bit (m_relations.m_names, SSA_NAME_VERSION (ssa1));
   bitmap_set_bit (m_relations.m_names, SSA_NAME_VERSION (ssa2));
   relation_chain *ptr = (relation_chain *) obstack_alloc (&m_chain_obstack,

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] PR tree-optimization/105458 - Check for equivalence after merging relations.
  2022-05-17 15:45 [PATCH] PR tree-optimization/105458 - Check for equivalence after merging relations Andrew MacLeod
@ 2022-05-18  5:57 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2022-05-18  5:57 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: gcc-patches

On Tue, May 17, 2022 at 5:46 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Sorry, missed this one earlier.
>
> When we register a relation, such as LE_EXPR,  we first check if there
> is an existing relation that applies, and if so they are combined. We
> were checking if the relation being registered was an EQ_EXPR, and if
> so, invoked the equivalence oracle.
>
>   I was doing the check for EQ_EXPR first, then merging with any
> existing relation.   In this case, the merge resulted in transforming
> the LE_EXPR into an EQ_EXPR, but the check to invoke the
> equivalence_oracle had already been done, and we got to a place we
> shouldn't have. doh!
>
> The fix is to do the merge first, then check for EQ_EXPR.
>
> The patch is a hair different (due to VREL_*  renames in gcc13), so I've
> attached both patches.
>
> bootstraps on gcc12 and gcc13 with no regressions.  pushed on trunk.
>
> OK for GCC12?

OK

> Andrew

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-05-18  5:57 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-17 15:45 [PATCH] PR tree-optimization/105458 - Check for equivalence after merging relations Andrew MacLeod
2022-05-18  5:57 ` 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).