diff mbox

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

Message ID 20121101092756.GF1891@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 1, 2012, 9:27 a.m. UTC 
On Fri, Oct 26, 2012 at 04:30:41AM -0200, Alexandre Oliva wrote:
> From: Alexandre Oliva <lxoliva@fsfla.org>
> 
> for  gcc/ChangeLog
> 
> 	PR debug/54693
> 	* tree-ssa-threadedge.c (thread_around_empty_block): Copy
> 	debug temps from predecessor before threading.

As can be seen on test-tgmath2.i, unfortunately many failed attempts
to thread can lead into big DEBUG stmt duplication this way.

The following patch avoids that by avoiding to copy debug stmts for
vars that already have a debug stmt at the beginning of the successor bb.
That can happen either from earlier thread_around_empty_block calls, or
just happen by accident, or even if the predecessor bb has several debug
stmts for the same var like
  DEBUG i => i_7
  i_8 = i_7 + 1
  DEBUG i => i_8
  i_9 = i_9 + 1
  DEBUG i => i_9
We'd needlessly copy over all debug stmts
  DEBUG i => i_7
  DEBUG i => i_8
  DEBUG i => i_9
to the succ bb.  Even for stmt frontiers it is IMHO undesirable to duplicate
(perhaps many times) the whole sequence, as one would then reply the var
changing sequence in the debugger perhaps once in the original bb, then
once or many times again in the successor bb.

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.

2012-11-01  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

Comments

Alexandre Oliva Nov. 2, 2012, 5:56 p.m. UTC | #1 
On Nov  1, 2012, Jakub Jelinek <jakub@redhat.com> wrote:

> Even for stmt frontiers it is IMHO undesirable to duplicate
> (perhaps many times) the whole sequence, as one would then reply the var
> changing sequence in the debugger perhaps once in the original bb, then
> once or many times again in the successor bb.

Quite the opposite: when we do jump threading, we're *bypassing* the
entire block, so in order to retain proper history we'd have to
propagate all the source-expected changes into the short-circuited
path.  To wit, we start with a computation log like this:

  jump to block A

A:
  # set foo to expr1
  # advance to line n
  # set foo to expr2
  jump to block B

B:
  # advance to line m
  # set foo to expr3

in which we can see foo bound to all 3 expressions, to a computation log
like this:

  jump to block B

B:
  # advance to line m
  # set foo to expr3

or:

  jump to block B

B:
  # set foo to expr1
  # advance to line n
  # set foo to expr2
  # advance to line m
  # set foo to expr3

which ones seems like a more faithful representation of the computation
described in the source code to you?
diff mbox

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,61 @@  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
+	      && 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.  */