diff mbox

[tree-optimization/71403] Do not allow threading to a deeper loop nest

Message ID b82c62ad-2767-5b6c-f8f8-f6ac28216d6a@redhat.com
State New
Headers show

Commit Message

Jeff Law June 13, 2016, 9 p.m. UTC
pr71403 (and its duplicates) show a problem where we thread a backedge 
from an outer loop to the header of an inner loop.

This looks all find and good at the CFG level, but it essentially 
combines the inner and outer loop with parts of the loop executing on 
some iterations, but not on others.  Worse yet, that will change the 
number of iterations of the loop, which wrecks havoc with the unroller.

This patch avoids those jumps threads.  It was also a good time to pull 
some path stack management out of a routine where it did not belong 
(convert_and_register_jump_thread_path).

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

jeff
commit 018f8824b168a5719defb8974efd110777b6b83b
Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Mon Jun 13 20:55:59 2016 +0000

    	PR tree-optimization/71403
    	* tree-ssa-threadbackward.c
    	(convert_and_register_jump_thread_path): No longer accept reference
    	to path.  Do not pop items off the path anymore.
    	(fsm_find_control_statement_thread_paths): Do not allow threading
    	to a deeper loop nest.  Pop the last item off the path here rather
    	than in convert_and_register_jump_thread_path.
    
    	PR tree-optimization/71403
    	* c-c++-common/ubsan/pr71403-1.c: New test.
    	* c-c++-common/ubsan/pr71403-2.c: New test.
    	* c-c++-common/ubsan/pr71403-3.c: New test.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@237403 138bc75d-0d04-0410-961f-82ee72b054a4
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 822e36f..91befb5 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@ 
+2016-06-13  Jeff Law  <law@redhat.com>
+
+	PR tree-optimization/71403
+	* tree-ssa-threadbackward.c
+	(convert_and_register_jump_thread_path): No longer accept reference
+	to path.  Do not pop items off the path anymore.
+	(fsm_find_control_statement_thread_paths): Do not allow threading
+	to a deeper loop nest.  Pop the last item off the path here rather
+	than in convert_and_register_jump_thread_path.
+
 2016-06-13  Kelvin Nilsen  <kelvin@gcc.gnu.org>
 
 	* config/rs6000/rs6000.h (RS6000_BTM_COMMON): Add the
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index d0dc9b7..6ba9050 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,10 @@ 
+2016-06-13  Jeff Law  <law@redhat.com>
+
+	PR tree-optimization/71403
+	* c-c++-common/ubsan/pr71403-1.c: New test.
+	* c-c++-common/ubsan/pr71403-2.c: New test.
+	* c-c++-common/ubsan/pr71403-3.c: New test.
+
 2016-06-13  Jakub Jelinek  <jakub@redhat.com>
 
 	PR middle-end/71478
diff --git a/gcc/testsuite/c-c++-common/ubsan/pr71403-1.c b/gcc/testsuite/c-c++-common/ubsan/pr71403-1.c
new file mode 100644
index 0000000..f8f4867
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/pr71403-1.c
@@ -0,0 +1,28 @@ 
+/* { dg-do run } */
+/* { dg-options "-O3 -fsanitize=unreachable" } */
+
+char a = -97;
+int b, c, d, e;
+
+int
+main ()
+{
+  int g = d, h = 0, i = 1; 
+  for (; h < 3; h++)
+    {
+      if (g > -1)
+        {
+          int j;
+          g = j = 0;
+          for (; j < 5; j++)
+          L1:
+            if (!i)
+              goto L1;
+          a = e;
+        }
+      else
+        i = 0;
+    }
+  b = c / ~(a | 114);
+  __builtin_exit (0);
+}
diff --git a/gcc/testsuite/c-c++-common/ubsan/pr71403-2.c b/gcc/testsuite/c-c++-common/ubsan/pr71403-2.c
new file mode 100644
index 0000000..03b6e83
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/pr71403-2.c
@@ -0,0 +1,22 @@ 
+/* { dg-do run } */
+/* { dg-options "-O3 -fsanitize=unreachable" } */
+
+char a, c;
+short b;
+
+int
+main ()
+{
+  unsigned d = 0;
+  int e = 1;
+  for (a = 0; a < 2; a++)
+    {
+      if (e)
+        c--;
+      for (; d < 2; d++)
+        for (b = 0; b; b++)
+          ;
+      e = 0;
+    }
+  __builtin_exit (0);
+}
diff --git a/gcc/testsuite/c-c++-common/ubsan/pr71403-3.c b/gcc/testsuite/c-c++-common/ubsan/pr71403-3.c
new file mode 100644
index 0000000..1ab7736
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/pr71403-3.c
@@ -0,0 +1,31 @@ 
+/* { dg-do run } */
+/* { dg-options "-O3 -fsanitize=unreachable" } */
+
+
+int a, b, c, d;
+
+void
+fn1 ()
+{
+  for (c = 0; c < 2; c++)
+    {
+      int e, f = 1;
+      for (e = 0; e < 2; e++)
+	{
+	  if (!f)
+	    return;
+	  for (d = 0; d; d++)
+	    f = b;
+	}
+    }
+}
+
+int
+main ()
+{
+  for (; a < 1; a++)
+    {
+      fn1 ();
+    }
+  __builtin_exit (0);
+}
diff --git a/gcc/tree-ssa-threadbackward.c b/gcc/tree-ssa-threadbackward.c
index 139d376..9dd37ad 100644
--- a/gcc/tree-ssa-threadbackward.c
+++ b/gcc/tree-ssa-threadbackward.c
@@ -378,7 +378,7 @@  profitable_jump_thread_path (vec<basic_block, va_gc> *&path,
    register the path.   */
 
 static void
-convert_and_register_jump_thread_path (vec<basic_block, va_gc> *&path,
+convert_and_register_jump_thread_path (vec<basic_block, va_gc> *path,
 				       edge taken_edge)
 {
   vec<jump_thread_edge *> *jump_thread_path = new vec<jump_thread_edge *> ();
@@ -402,9 +402,6 @@  convert_and_register_jump_thread_path (vec<basic_block, va_gc> *&path,
 
   register_jump_thread (jump_thread_path);
   --max_threaded_paths;
-
-  /* Remove BBI from the path.  */
-  path->pop ();
 }
 
 /* We trace the value of the SSA_NAME NAME back through any phi nodes looking
@@ -459,6 +456,9 @@  fsm_find_control_statement_thread_paths (tree name,
       seen_loop_phi = true;
     }
 
+  if (bb_loop_depth (last_bb_in_path) > bb_loop_depth (var_bb))
+    return;
+
   /* Following the chain of SSA_NAME definitions, we jumped from a definition in
      LAST_BB_IN_PATH to a definition in VAR_BB.  When these basic blocks are
      different, append to PATH the blocks from LAST_BB_IN_PATH to VAR_BB.  */
@@ -505,7 +505,9 @@  fsm_find_control_statement_thread_paths (tree name,
 	 NEXT_PATH.  Don't add them here to avoid pollution.  */
       for (unsigned int i = 0; i < next_path->length () - 1; i++)
 	{
-	  if (visited_bbs->contains ((*next_path)[i]))
+	  if (visited_bbs->contains ((*next_path)[i])
+	      || (bb_loop_depth (last_bb_in_path)
+		  > bb_loop_depth ((*next_path)[i])))
 	    {
 	      vec_free (next_path);
 	      return;
@@ -557,7 +559,12 @@  fsm_find_control_statement_thread_paths (tree name,
 	     into the canonical form and register it.  */
 	  edge taken_edge = profitable_jump_thread_path (path, bbi, name, arg);
 	  if (taken_edge)
-	    convert_and_register_jump_thread_path (path, taken_edge);
+	    {
+	      if (bb_loop_depth (taken_edge->src)
+		  >= bb_loop_depth (taken_edge->dest))
+		convert_and_register_jump_thread_path (path, taken_edge);
+	      path->pop ();
+	    }
 	}
     }
   else if (gimple_code (def_stmt) == GIMPLE_ASSIGN)
@@ -578,7 +585,12 @@  fsm_find_control_statement_thread_paths (tree name,
 	  edge taken_edge = profitable_jump_thread_path (path, var_bb,
 						     name, arg);
 	  if (taken_edge)
-	    convert_and_register_jump_thread_path (path, taken_edge);
+	    {
+	      if (bb_loop_depth (taken_edge->src)
+		  >= bb_loop_depth (taken_edge->dest))
+		convert_and_register_jump_thread_path (path, taken_edge);
+	      path->pop ();
+	    }
 
 	  /* And put the current block back onto the path so that the
 	     state of the stack is unchanged when we leave.  */