Patchwork [PR54693] loss of debug info in jump threading and loop ivopts

login
register
mail settings
Submitter Jakub Jelinek
Date Nov. 2, 2012, 8:08 a.m.
Message ID <20121102080845.GB1881@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/196508/
State New
Headers show

Comments

Jakub Jelinek - Nov. 2, 2012, 8:08 a.m.
On Thu, Nov 01, 2012 at 10:27:57AM +0100, Jakub Jelinek wrote:
> The patch unfortunately doesn't speed test-tgmath2.i compilation
> significantly, but decreases number of debug stmts from ~ 36000 to ~ 16000,
> the 20000+ were clearly redundant.

Testing found a buglet in the patch (doing is_gimple_debug (gsi_stmt (gsi))
even when there might be no stmts after labels in the successor at all),
fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?

2012-11-02  Jakub Jelinek  <jakub@redhat.com>

	PR debug/54402
	* tree-ssa-threadedge.c (thread_around_empty_block): Don't copy over
	debug stmts if the successor bb already starts with a debug stmt for
	the same var.


	Jakub
Jeff Law - Nov. 2, 2012, 5:13 p.m.
On 11/02/2012 02:08 AM, Jakub Jelinek wrote:
> On Thu, Nov 01, 2012 at 10:27:57AM +0100, Jakub Jelinek wrote:
>> The patch unfortunately doesn't speed test-tgmath2.i compilation
>> significantly, but decreases number of debug stmts from ~ 36000 to ~ 16000,
>> the 20000+ were clearly redundant.
>
> Testing found a buglet in the patch (doing is_gimple_debug (gsi_stmt (gsi))
> even when there might be no stmts after labels in the successor at all),
> fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk?
>
> 2012-11-02  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR debug/54402
> 	* tree-ssa-threadedge.c (thread_around_empty_block): Don't copy over
> 	debug stmts if the successor bb already starts with a debug stmt for
> 	the same var.
OK.
Jeff

Patch

--- gcc/tree-ssa-threadedge.c.jj	2012-10-30 09:01:15.000000000 +0100
+++ gcc/tree-ssa-threadedge.c	2012-11-01 10:07:29.114499218 +0100
@@ -1,5 +1,5 @@ 
 /* SSA Jump Threading
-   Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010, 2011
+   Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012
    Free Software Foundation, Inc.
    Contributed by Jeff Law  <law@redhat.com>
 
@@ -641,18 +641,62 @@  thread_around_empty_block (edge taken_ed
      we don't lose the bindings as we redirect the edges.  */
   if (MAY_HAVE_DEBUG_STMTS)
     {
+      struct pointer_set_t *vars = NULL;
+
       gsi = gsi_after_labels (bb);
       for (gimple_stmt_iterator si = gsi_last_bb (taken_edge->src);
 	   !gsi_end_p (si); gsi_prev (&si))
 	{
+	  tree var;
+
 	  stmt = gsi_stmt (si);
 	  if (!is_gimple_debug (stmt))
 	    continue;
 
+	  var = NULL_TREE;
+	  if (gimple_debug_bind_p (stmt))
+	    var = gimple_debug_bind_get_var (stmt);
+	  else if (gimple_debug_source_bind_p (stmt))
+	    var = gimple_debug_source_bind_get_var (stmt);
+
+	  /* Don't insert debug stmts for vars which will be immediately
+	     overridden by debug stmts at the start of bb (either preexisting
+	     or from earlier thread_around_empty_block attempt), without
+	     any intervening real stmts.  */
+	  if (var != NULL_TREE
+	      && !gsi_end_p (gsi)
+	      && is_gimple_debug (gsi_stmt (gsi)))
+	    {
+	      if (vars == NULL)
+		{
+		  gimple_stmt_iterator gsi2;
+		  vars = pointer_set_create ();
+		  for (gsi2 = gsi; !gsi_end_p (gsi2); gsi_next (&gsi2))
+		    {
+		      gimple stmt2 = gsi_stmt (gsi2);
+		      tree var2;
+		      if (!is_gimple_debug (stmt2))
+			break;
+		      if (gimple_debug_bind_p (stmt2))
+			var2 = gimple_debug_bind_get_var (stmt2);
+		      else if (gimple_debug_source_bind_p (stmt2))
+			var2 = gimple_debug_source_bind_get_var (stmt2);
+		      else
+			continue;
+		      pointer_set_insert (vars, var2);
+		    }
+		}
+	      if (pointer_set_insert (vars, var))
+		continue;
+	    }
+
 	  stmt = gimple_copy (stmt);
 	  /* ??? Should we drop the location of the copy?  */
 	  gsi_insert_before (&gsi, stmt, GSI_NEW_STMT);
 	}
+
+      if (vars != NULL)
+	pointer_set_destroy (vars);
     }
 
   /* This block must have more than one successor.  */