diff mbox

[PR,tree-optimization/59322] Remove unwanted jump thread path copying

Message ID 529D53EB.6000309@redhat.com
State New
Headers show

Commit Message

Jeff Law Dec. 3, 2013, 3:45 a.m. UTC 
Code was added to copy the jump threading path (AUX field on an edge) by 
a change from Zdenek in 2007.  At the time the code was added, AFAICT, 
the copied AUX field would never be examined and certainly not used for 
threading.

I'd been suspicious of Zdenek's code to copy the AUX field, but went 
along with it and changed it to properly copy the updated representation.

The testcase for 59322 is wonderful in that it shows how that copying is 
just plan bad because it results in dangling embedded edge pointers in 
the jump threading path.    We make the copy and attach it to a new 
edge's AUX field in the CFG.  We then thread from outside the loop 
through the loop header.  We can delete edges in the CFG as a result. 
This leaves a dangling edge pointer in a the jump threading path 
structure (particularly the copied one).

We then walk all the edges to see if any are threads from inside the 
loop, through the backedge to another point within the loop.  This may 
reference the jump threading path containing the dangling edge pointer.

As I mentioned, I went back and looked at the 2007 code and AFAICT 
copying the AUX field was dead when it was added.  This patch removes 
the code to copy the jump threading path in the AUX field to newly 
created edges.  And (of course), it fixes the 59322 testcase.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu. 
Installed on the trunk.

Jeff
PR tree-optimization/59322
	* tree-ssa-threadedge.c (create_edge_and_update_destination_phis):
	Remove code which copied jump threading paths.

	PR tree-optimization/59322
	* gcc.c-torture/compile/pr59322.c: New test
diff mbox

Patch

diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
index 24d0f42..ad727a1 100644
--- a/gcc/tree-ssa-threadupdate.c
+++ b/gcc/tree-ssa-threadupdate.c
@@ -421,27 +421,22 @@  create_edge_and_update_destination_phis (struct redirection_data *rd,
   e->probability = REG_BR_PROB_BASE;
   e->count = bb->count;
 
-  /* We have to copy path -- which means creating a new vector as well
-     as all the jump_thread_edge entries.  */
-  if (rd->path->last ()->e->aux)
-    {
-      vec<jump_thread_edge *> *path = THREAD_PATH (rd->path->last ()->e);
-      vec<jump_thread_edge *> *copy = new vec<jump_thread_edge *> ();
+  /* We used to copy the thread path here.  That was added in 2007
+     and dutifully updated through the representation changes in 2013.
 
-      /* Sadly, the elements of the vector are pointers and need to
-	 be copied as well.  */
-      for (unsigned int i = 0; i < path->length (); i++)
-	{
-	  jump_thread_edge *x
-	    = new jump_thread_edge ((*path)[i]->e, (*path)[i]->type);
-	  copy->safe_push (x);
-	}
-      e->aux = (void *)copy;
-    }
-  else
-    {
-      e->aux = NULL;
-    }
+     In 2013 we added code to thread from an interior node through
+     the backedge to another interior node.  That runs after the code
+     to thread through loop headers from outside the loop.
+
+     The latter may delete edges in the CFG, including those
+     which appeared in the jump threading path we copied here.  Thus
+     we'd end up using a dangling pointer.
+
+     After reviewing the 2007/2011 code, I can't see how anything
+     depended on copying the AUX field and clearly copying the jump
+     threading path is problematical due to embedded edge pointers.
+     It has been removed.  */
+  e->aux = NULL;
 
   /* If there are any PHI nodes at the destination of the outgoing edge
      from the duplicate block, then we will need to add a new argument