[committed,PR,tree-optimization/83477] Always record a result range for PHIs

Message ID 71ed5116-7b31-a0c7-75ba-fb1aa861d314@redhat.com
State New
Headers show
Series
  • [committed,PR,tree-optimization/83477] Always record a result range for PHIs
Related show

Commit Message

Jeff Law Dec. 19, 2017, 8:13 p.m.
BZ83477 is another case where we had an VR_UNDEFINED range for source
operand during jump threading.

In this instance we had a PHI argument that was a constant.  During
threading we record that the PHI result has the constant as its value.

I mistakenly thought that would avoid the need to mess around with the
value range unwinding stack.  After all if the PHI result has a constant
value, we can just use the constant value.

But we handle generation of output ranges for a given statement before
we do const/copy propagation and simplification.

So we used the VR_UNDEFINED range for the condition in a COND_EXPR rhs
of a gimple assignment.  This resulted in an incorrect output range for
the assignment that was later used in other jump threading decisions.

The fix is to always push a range for a non-virtual PHI during jump
threading.  WHen the source operand is an SSA_NAME, we can just copy its
range.  When the source operand is an INTEGER_CST, we can create a range
from that.  Otherwise we use VR_VARYING.

I'd hoped this would fix the 435.gromacs problem, but it doesn't appear
to do so :(

Bootstrapped and regression tested on x86_64.  Installing on the trunk.

Jeff
commit d69c59491a47db627c2429abef7547e7f005fc40
Author: Jeff Law <law@torsion.usersys.redhat.com>
Date:   Tue Dec 19 15:07:12 2017 -0500

            PR tree-optimization/83477
            * tree-ssa-threadedge.c (record_temporary_equivalences_from_phis): For
            a non-virtual PHI, always push a new range.
    
            PR tree-optimization/83477
            * gcc.c-torture/execute/pr83477.c: New test.

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 4581f41d8fb..8a80f18045b 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@ 
+2017-12-18  Jeff Law  <law@redhat.com>
+
+	PR tree-optimization/83477
+	* tree-ssa-threadedge.c (record_temporary_equivalences_from_phis): For
+	a non-virtual PHI, always push a new range.
+
 2017-12-19  Martin Sebor  <msebor@redhat.com>
 
 	PR middle-end/77608
@@ -195,7 +201,6 @@ 
 	* tree-ssa-dom.c (record_equivalences_from_phis): Fix handling
 	of degenerates resulting from ignoring an edge.
 
->>>>>>> .r255835
 2017-12-18  Martin Sebor  <msebor@redhat.com>
 
 	PR middle-end/83373
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 63d671374e3..b32cb7a09fa 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2017-12-18  Jeff Law  <law@redhat.com>
+
+	PR tree-optimization/83477
+	* gcc.c-torture/execute/pr83477.c: New test.
+
 2017-12-19  Martin Sebor  <msebor@redhat.com>
 
 	PR middle-end/77608
@@ -63,7 +68,6 @@ 
 	PR ipa/83346
 	* g++.dg/ipa/pr82801.C: New test.
 
->>>>>>> .r255835
 2017-12-18  Martin Sebor  <msebor@redhat.com>
 
 	PR middle-end/83373
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr83477.c b/gcc/testsuite/gcc.c-torture/execute/pr83477.c
new file mode 100644
index 00000000000..de667fd7d68
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr83477.c
@@ -0,0 +1,22 @@ 
+int yf = 0;
+
+void
+pl (int q5, int nd)
+{
+  unsigned int hp = q5;
+  int zx = (q5 == 0) ? hp : (hp / q5);
+
+  yf = ((nd < 2) * zx != 0) ? nd : 0;
+}
+
+int
+main (void)
+{
+  pl (1, !yf);
+  if (yf != 1)
+    __builtin_abort ();
+
+  return 0;
+}
+
+
diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c
index 1fafd7b5932..0c782f51697 100644
--- a/gcc/tree-ssa-threadedge.c
+++ b/gcc/tree-ssa-threadedge.c
@@ -156,11 +156,37 @@  record_temporary_equivalences_from_phis (edge e,
       const_and_copies->record_const_or_copy (dst, src);
 
       /* Also update the value range associated with DST, using
-	 the range from SRC.  */
-      if (evrp_range_analyzer && TREE_CODE (src) == SSA_NAME)
+	 the range from SRC.
+
+	 Note that even if SRC is a constant we need to set a suitable
+	 output range so that VR_UNDEFINED ranges do not leak through.  */
+      if (evrp_range_analyzer)
 	{
-	  value_range *vr = evrp_range_analyzer->get_value_range (src);
-	  evrp_range_analyzer->push_value_range (dst, vr);
+	  /* Get an empty new VR we can pass to update_value_range and save
+	     away in the VR stack.  */
+	  vr_values *vr_values = evrp_range_analyzer->get_vr_values ();
+	  value_range *new_vr = vr_values->allocate_value_range ();
+	  memset (new_vr, 0, sizeof (value_range));
+
+	  /* There are three cases to consider:
+
+	       First if SRC is an SSA_NAME, then we can copy the value
+	       range from SRC into NEW_VR.
+
+	       Second if SRC is an INTEGER_CST, then we can just wet
+	       NEW_VR to a singleton range.
+
+	       Otherwise set NEW_VR to varying.  This may be overly
+	       conservative.  */
+	  if (TREE_CODE (src) == SSA_NAME)
+	    copy_value_range (new_vr, vr_values->get_value_range (src));
+	  else if (TREE_CODE (src) == INTEGER_CST)
+	    set_value_range_to_value (new_vr, src,  NULL);
+	  else
+	    set_value_range_to_varying (new_vr);
+
+	  /* This is a temporary range for DST, so push it.  */
+	  evrp_range_analyzer->push_value_range (dst, new_vr);
 	}
     }
   return true;