diff mbox series

tree-optimization/103440 - Always track arguments, even when ignoring equiv params.

Message ID 31e03986-11ba-6c53-7ce0-1636eb25a7e7@redhat.com
State New
Headers show
Series tree-optimization/103440 - Always track arguments, even when ignoring equiv params. | expand

Commit Message

Andrew MacLeod Nov. 29, 2021, 5:20 p.m. UTC 
I need to adjust the original patch, I shouldn't have continued the loop 
when dealing with equivalences.    An equivalence is not the same as an 
undefined value...  we might be able to ignore the range from it for 
calculation purposes, but we cannot ignore the fact that it is a 
different SSA_NAME and may contain a different value that we do care about.

There are other checks in then loop which will allow us to assign an 
equivalence between the DEF and an argument if we are ignoring the other 
ssa_names..  such as when there are undefined values.

a_3 = PHI <a_1(4), a_2(6)>

if a_2 is undefined, we can create an equivalence between a_3 and a_1 as 
the value of a_2 is irrelevant and can be whatever we want it to be.

if a_2 is instead an equivalence with a_3, we do not want to create an 
equivalence between a_3 and a_1 in this block as we may then turn it 
into a copy.. we'd only be able to do this if there was an equivalence 
between a_1 and a_2, and we are not checking that.

Although we are may not be adding the range for a_2 into the cumulative 
knowledge of a_3's range, we do need to keep the edge to retain the copy 
as its value is important and could be different than the other 
argument... and we need to retain the copy when we go out of ssa.

This fixes that oversight, bootstrapped on x86_64-pc-linux-gnu with no 
regressions.  OK?

Caveat..  the test case has an infinite loop without this fix, but 
degagnu doesn't seem to kill it, and my test suite runs go forever.. if 
there something I am missing?   The gcc.log claims that it timeouts 
after 300 second, but it doesn't kill the executable. I set the 
dg-timeout field, but that appears to be a compile time timeout, not 
runtime anyway.

Andrew

Comments

Richard Biener Nov. 30, 2021, 7:42 a.m. UTC | #1 
On Mon, Nov 29, 2021 at 6:20 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> I need to adjust the original patch, I shouldn't have continued the loop
> when dealing with equivalences.    An equivalence is not the same as an
> undefined value...  we might be able to ignore the range from it for
> calculation purposes, but we cannot ignore the fact that it is a
> different SSA_NAME and may contain a different value that we do care about.
>
> There are other checks in then loop which will allow us to assign an
> equivalence between the DEF and an argument if we are ignoring the other
> ssa_names..  such as when there are undefined values.
>
> a_3 = PHI <a_1(4), a_2(6)>
>
> if a_2 is undefined, we can create an equivalence between a_3 and a_1 as
> the value of a_2 is irrelevant and can be whatever we want it to be.
>
> if a_2 is instead an equivalence with a_3, we do not want to create an
> equivalence between a_3 and a_1 in this block as we may then turn it
> into a copy.. we'd only be able to do this if there was an equivalence
> between a_1 and a_2, and we are not checking that.
>
> Although we are may not be adding the range for a_2 into the cumulative
> knowledge of a_3's range, we do need to keep the edge to retain the copy
> as its value is important and could be different than the other
> argument... and we need to retain the copy when we go out of ssa.
>
> This fixes that oversight, bootstrapped on x86_64-pc-linux-gnu with no
> regressions.  OK?

OK.

> Caveat..  the test case has an infinite loop without this fix, but
> degagnu doesn't seem to kill it, and my test suite runs go forever.. if
> there something I am missing?   The gcc.log claims that it timeouts
> after 300 second, but it doesn't kill the executable. I set the
> dg-timeout field, but that appears to be a compile time timeout, not
> runtime anyway.

Might be an old issue with your dejagnu, it should work.

Richard.

>
> Andrew
diff mbox series

Patch

From 2991aa06aaaf87874df8ed1c93a4650dc85c79ec Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Mon, 29 Nov 2021 09:19:34 -0500
Subject: [PATCH 1/2] Always track arguments, even when ignoring equiv params.

To "ignore" ranges from equivalences, we should track the range separately,
but still do the other name processing which determiens if there is a single
name or not for equivalence.  Otherwise we mistakently think we can introduce
an equivalences.

	gcc/
	PR tree-optimization/103440
	* gimple-range-fold.cc (fold_using_range::range_of_phi): Continue
	normal param processing for equiv params.

	gcc/testsuite/
	* gcc.dg/pr103440.c: New.
---
 gcc/gimple-range-fold.cc        | 21 +++++++++------------
 gcc/testsuite/gcc.dg/pr103440.c | 24 ++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr103440.c

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index d66ada5bb7c..58122297c0b 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -795,20 +795,17 @@  fold_using_range::range_of_phi (irange &r, gphi *phi, fur_source &src)
       // Get the range of the argument on its edge.
       src.get_phi_operand (arg_range, arg, e);
 
-      // Likewise, if the incoming PHI argument is equivalent to this
-      // PHI definition, it provides no new info.  Accumulate these ranges
-      // in case all arguments are equivalences.
-      if (src.query ()->query_relation (e, arg, phi_def, false) == EQ_EXPR)
-	{
-	  single_arg = arg;
-	  equiv_range.union_(arg_range);
-	  continue;
-	}
-
       if (!arg_range.undefined_p ())
 	{
 	  // Register potential dependencies for stale value tracking.
-	  r.union_ (arg_range);
+	  // Likewise, if the incoming PHI argument is equivalent to this
+	  // PHI definition, it provides no new info.  Accumulate these ranges
+	  // in case all arguments are equivalences.
+	  if (src.query ()->query_relation (e, arg, phi_def, false) == EQ_EXPR)
+	    equiv_range.union_(arg_range);
+	  else
+	    r.union_ (arg_range);
+
 	  if (gimple_range_ssa_p (arg) && src.gori ())
 	    src.gori ()->register_dependency (phi_def, arg);
 
@@ -829,7 +826,7 @@  fold_using_range::range_of_phi (irange &r, gphi *phi, fur_source &src)
 
     // If all arguments were equivalences, use the equivalence ranges as no
     // arguments were processed.
-    if (!seen_arg)
+    if (r.undefined_p () && !equiv_range.undefined_p ())
       r = equiv_range;
 
     // If the PHI boils down to a single effective argument, look at it.
diff --git a/gcc/testsuite/gcc.dg/pr103440.c b/gcc/testsuite/gcc.dg/pr103440.c
new file mode 100644
index 00000000000..b97f45cd3ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr103440.c
@@ -0,0 +1,24 @@ 
+/* { dg-do run } */
+/* { dg-options "-Os" } */
+/* { dg-timeout 10 } */
+
+int a, b, c, d, e;
+int main() {
+  int f = 2, g = 1, h = -3;
+L1:
+  c = b ^ 1;
+  if (!f)
+    goto L3;
+  if (d)
+    g = e;
+  f = h;
+  if (!c)
+    goto L1;
+L2:
+  if (g)
+    a = 0;
+L3:
+  if (d == g)
+    goto L2;
+  return 0;
+}
-- 
2.17.2