diff mbox

Improve debug info for partial inlining (PR debug/54519)

Message ID 20120911135956.GP22619@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Sept. 11, 2012, 1:59 p.m. UTC 
Hi!

As discussed in the PR, right now we do a very bad job for debug info
of partially inlined functions (both when they are kept only partially
inlined, or when partial inlining is performed, but doesn't seem to be
useful and foo.part.N is inlined back, either into the original function, or
into a function into which foo has been inlined first).

This patch improves that by doing something similar to what ipa-prop.c does,
in particular for arguments that aren't actually passed to foo.part.N
we add debug args and corresponding debug bind and debug source bind stmts
to provide better debug info (if foo.part.N isn't inlined, then
DW_OP_GNU_parameter_ref is going to be used together with corresponding call
site arguments).

Bootstrapped/regtested on x86_64-linux and i686-linux, some of the tests
still fail with some option combinations, am going to file a DF VTA PR for
that momentarily.  Ok for trunk?

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

	PR debug/54519
	* ipa-split.c (split_function): Add debug args and
	debug source and normal stmts for args_to_skip which are
	gimple regs.
	* tree-inline.c (copy_debug_stmt): When inlining, adjust
	source debug bind stmts to debug binds of corresponding
	DEBUG_EXPR_DECL.

	* gcc.dg/guality/pr54519-1.c: New test.
	* gcc.dg/guality/pr54519-2.c: New test.
	* gcc.dg/guality/pr54519-3.c: New test.
	* gcc.dg/guality/pr54519-4.c: New test.
	* gcc.dg/guality/pr54519-5.c: New test.


	Jakub

Comments

Steven Bosscher Sept. 11, 2012, 2:41 p.m. UTC | #1 
> +  if (args_to_skip)
> +    for (parm = DECL_ARGUMENTS (current_function_decl), num = 0;
> +        parm; parm = DECL_CHAIN (parm), num++)
> +      if (bitmap_bit_p (args_to_skip, num)
> +         && is_gimple_reg (parm))
> +       {
> +         tree ddecl;
> +         gimple def_temp;
> +
> +         arg = get_or_create_ssa_default_def (cfun, parm);
> +         if (!MAY_HAVE_DEBUG_STMTS)
> +           continue;

You can do this MAY_HAVE_DEBUG_STMTS check before the loop, e.g.

> +  if (args_to_skip && MAY_HAVE_DEBUG_STMTS)

Ciao!
Steven
Jakub Jelinek Sept. 11, 2012, 2:47 p.m. UTC | #2 
On Tue, Sep 11, 2012 at 04:41:24PM +0200, Steven Bosscher wrote:
> > +  if (args_to_skip)
> > +    for (parm = DECL_ARGUMENTS (current_function_decl), num = 0;
> > +        parm; parm = DECL_CHAIN (parm), num++)
> > +      if (bitmap_bit_p (args_to_skip, num)
> > +         && is_gimple_reg (parm))
> > +       {
> > +         tree ddecl;
> > +         gimple def_temp;
> > +
> > +         arg = get_or_create_ssa_default_def (cfun, parm);
> > +         if (!MAY_HAVE_DEBUG_STMTS)
> > +           continue;
> 
> You can do this MAY_HAVE_DEBUG_STMTS check before the loop, e.g.
> 
> > +  if (args_to_skip && MAY_HAVE_DEBUG_STMTS)

No, that would result in -fcompare-debug failures if parm doesn't have a
default def yet.

	Jakub
diff mbox

Patch

--- gcc/ipa-split.c.jj	2012-08-20 11:09:45.000000000 +0200
+++ gcc/ipa-split.c	2012-09-10 16:04:39.499558177 +0200
@@ -1059,6 +1059,7 @@  split_function (struct split_point *spli
   gimple last_stmt = NULL;
   unsigned int i;
   tree arg, ddef;
+  VEC(tree, gc) **debug_args = NULL;
 
   if (dump_file)
     {
@@ -1232,6 +1233,65 @@  split_function (struct split_point *spli
   gimple_set_block (call, DECL_INITIAL (current_function_decl));
   VEC_free (tree, heap, args_to_pass);
 
+  if (args_to_skip)
+    for (parm = DECL_ARGUMENTS (current_function_decl), num = 0;
+	 parm; parm = DECL_CHAIN (parm), num++)
+      if (bitmap_bit_p (args_to_skip, num)
+	  && is_gimple_reg (parm))
+	{
+	  tree ddecl;
+	  gimple def_temp;
+
+	  arg = get_or_create_ssa_default_def (cfun, parm);
+	  if (!MAY_HAVE_DEBUG_STMTS)
+	    continue;
+	  if (debug_args == NULL)
+	    debug_args = decl_debug_args_insert (node->symbol.decl);
+	  ddecl = make_node (DEBUG_EXPR_DECL);
+	  DECL_ARTIFICIAL (ddecl) = 1;
+	  TREE_TYPE (ddecl) = TREE_TYPE (parm);
+	  DECL_MODE (ddecl) = DECL_MODE (parm);
+	  VEC_safe_push (tree, gc, *debug_args, parm);
+	  VEC_safe_push (tree, gc, *debug_args, ddecl);
+	  def_temp = gimple_build_debug_bind (ddecl, unshare_expr (arg),
+					      call);
+	  gsi_insert_after (&gsi, def_temp, GSI_NEW_STMT);
+	}
+  if (debug_args != NULL)
+    {
+      unsigned int i;
+      tree var, vexpr;
+      gimple_stmt_iterator cgsi;
+      gimple def_temp;
+
+      push_cfun (DECL_STRUCT_FUNCTION (node->symbol.decl));
+      var = BLOCK_VARS (DECL_INITIAL (node->symbol.decl));
+      i = VEC_length (tree, *debug_args);
+      cgsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR));
+      do
+	{
+	  i -= 2;
+	  while (var != NULL_TREE
+		 && DECL_ABSTRACT_ORIGIN (var)
+		    != VEC_index (tree, *debug_args, i))
+	    var = TREE_CHAIN (var);
+	  if (var == NULL_TREE)
+	    break;
+	  vexpr = make_node (DEBUG_EXPR_DECL);
+	  parm = VEC_index (tree, *debug_args, i);
+	  DECL_ARTIFICIAL (vexpr) = 1;
+	  TREE_TYPE (vexpr) = TREE_TYPE (parm);
+	  DECL_MODE (vexpr) = DECL_MODE (parm);
+	  def_temp = gimple_build_debug_source_bind (vexpr, parm,
+						     NULL);
+	  gsi_insert_before (&cgsi, def_temp, GSI_SAME_STMT);
+	  def_temp = gimple_build_debug_bind (var, vexpr, NULL);
+	  gsi_insert_before (&cgsi, def_temp, GSI_SAME_STMT);
+	}
+      while (i);
+      pop_cfun ();
+    }
+
   /* We avoid address being taken on any variable used by split part,
      so return slot optimization is always possible.  Moreover this is
      required to make DECL_BY_REFERENCE work.  */
--- gcc/tree-inline.c.jj	2012-08-22 11:18:56.000000000 +0200
+++ gcc/tree-inline.c	2012-09-11 09:13:49.509205799 +0200
@@ -2355,6 +2355,31 @@  copy_debug_stmt (gimple stmt, copy_body_
       gimple_debug_source_bind_set_var (stmt, t);
       walk_tree (gimple_debug_source_bind_get_value_ptr (stmt),
 		 remap_gimple_op_r, &wi, NULL);
+      /* When inlining and source bind refers to one of the optimized
+	 away parameters, change the source bind into normal debug bind
+	 referring to the corresponding DEBUG_EXPR_DECL that should have
+	 been bound before the call stmt.  */
+      t = gimple_debug_source_bind_get_value (stmt);
+      if (t != NULL_TREE
+	  && TREE_CODE (t) == PARM_DECL
+	  && id->gimple_call)
+	{
+	  VEC(tree, gc) **debug_args = decl_debug_args_lookup (id->src_fn);
+	  unsigned int i;
+	  if (debug_args != NULL)
+	    {
+	      for (i = 0; i < VEC_length (tree, *debug_args); i += 2)
+		if (VEC_index (tree, *debug_args, i) == t
+		    && TREE_CODE (VEC_index (tree, *debug_args, i + 1))
+		       == DEBUG_EXPR_DECL)
+		  {
+		    t = VEC_index (tree, *debug_args, i + 1);
+		    gimple_debug_source_bind_set_value (stmt, t);
+		    stmt->gsbase.subcode = GIMPLE_DEBUG_BIND;
+		    break;
+		  }
+	    }
+	}      
     }
 
   processing_debug_stmt = 0;
--- gcc/testsuite/gcc.dg/guality/pr54519-2.c.jj	2012-09-10 17:57:01.189624668 +0200
+++ gcc/testsuite/gcc.dg/guality/pr54519-2.c	2012-09-10 17:56:08.964869830 +0200
@@ -0,0 +1,45 @@ 
+/* PR debug/54519 */
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+__attribute__((noinline, noclone)) void
+fn1 (int x)
+{
+  __asm volatile ("" : "+r" (x) : : "memory");
+}
+
+static int
+fn2 (int x, int y)
+{
+  if (y)
+    {
+      fn1 (x);		/* { dg-final { gdb-test 17 "x" "6" } } */
+      fn1 (x);		/* { dg-final { gdb-test 17 "y" "25" } } */
+      fn1 (x);
+      fn1 (x);
+      y = -2 + x;
+      y = y * y * y + y;
+      fn1 (x + y);	/* { dg-final { gdb-test 22 "y" "68" } } */
+    }
+  return x;
+}
+
+__attribute__((noinline, noclone)) int
+fn3 (int x, int y)
+{
+  return fn2 (x, y) + y;
+}
+
+__attribute__((noinline, noclone)) int
+fn4 (int x, int y)
+{
+  return fn2 (x, y) + y;
+}
+
+int
+main ()
+{
+  fn3 (6, 25);
+  fn4 (4, 117);
+  return 0;
+}
--- gcc/testsuite/gcc.dg/guality/pr54519-1.c.jj	2012-09-10 17:56:54.532655918 +0200
+++ gcc/testsuite/gcc.dg/guality/pr54519-1.c	2012-09-10 17:56:03.205896869 +0200
@@ -0,0 +1,48 @@ 
+/* PR debug/54519 */
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+__attribute__((noinline, noclone)) void
+fn1 (int x)
+{
+  __asm volatile ("" : "+r" (x) : : "memory");
+}
+
+static int
+fn2 (int x, int y, int z)
+{
+  int a = 8;
+  if (x != z)
+    {
+      fn1 (x);
+      fn1 (x);		/* { dg-final { gdb-test 20 "x" "36" } } */
+      if (x == 36)	/* { dg-final { gdb-test 20 "y" "25" } } */
+	fn1 (x);	/* { dg-final { gdb-test 20 "z" "6" } } */
+      fn1 (x);		/* { dg-final { gdb-test 23 "x" "98" } } */
+      if (x == 98)	/* { dg-final { gdb-test 23 "y" "117" } } */
+	fn1 (x);	/* { dg-final { gdb-test 23 "z" "8" } } */
+      fn1 (x);
+      fn1 (x + a);
+    }
+  return y;
+}
+
+__attribute__((noinline, noclone)) int
+fn3 (int x, int y)
+{
+  return fn2 (x, y, 6);
+}
+
+__attribute__((noinline, noclone)) int
+fn4 (int x, int y)
+{
+  return fn2 (x, y, 8);
+}
+
+int
+main ()
+{
+  fn3 (36, 25);
+  fn4 (98, 117);
+  return 0;
+}
--- gcc/testsuite/gcc.dg/guality/pr54519-3.c.jj	2012-09-10 17:57:03.735612717 +0200
+++ gcc/testsuite/gcc.dg/guality/pr54519-3.c	2012-09-10 17:56:14.837842257 +0200
@@ -0,0 +1,42 @@ 
+/* PR debug/54519 */
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+__attribute__((noinline, noclone)) void
+fn1 (int x)
+{
+  __asm volatile ("" : "+r" (x) : : "memory");
+}
+
+static int
+fn2 (int x, int y, int z)
+{
+  int a = 8;
+  if (x != z)
+    {
+      fn1 (x);
+      fn1 (x);		/* { dg-final { gdb-test 20 "x" "36" } } */
+      if (x == 36)	/* { dg-final { gdb-test 20 "y" "25" } } */
+	fn1 (x);	/* { dg-final { gdb-test 20 "z" "6" } } */
+      fn1 (x);		/* { dg-final { gdb-test 23 "x" "98" } } */
+      if (x == 98)	/* { dg-final { gdb-test 23 "y" "117" } } */
+	fn1 (x);	/* { dg-final { gdb-test 23 "z" "8" } } */
+      fn1 (x);
+      fn1 (x + a);
+    }
+  return y;
+}
+
+int (*p) (int, int, int) = fn2;
+
+int
+main ()
+{
+  __asm volatile ("" : : : "memory");
+  int (*q) (int, int, int) = p;
+  __asm volatile ("" : : : "memory");
+  q (36, 25, 6);
+  q (98, 117, 8);
+  q (0, 0, 0);
+  return 0;
+}
--- gcc/testsuite/gcc.dg/guality/pr54519-4.c.jj	2012-09-10 17:57:05.915602485 +0200
+++ gcc/testsuite/gcc.dg/guality/pr54519-4.c	2012-09-10 17:55:56.811926879 +0200
@@ -0,0 +1,39 @@ 
+/* PR debug/54519 */
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+__attribute__((noinline, noclone)) void
+fn1 (int x)
+{
+  __asm volatile ("" : "+r" (x) : : "memory");
+}
+
+static int
+fn2 (int x, int y)
+{
+  if (y)
+    {
+      fn1 (x);		/* { dg-final { gdb-test 17 "x" "6" } } */
+      fn1 (x);		/* { dg-final { gdb-test 17 "y" "25" } } */
+      fn1 (x);
+      fn1 (x);
+      y = -2 + x;
+      y = y * y * y + y;
+      fn1 (x + y);	/* { dg-final { gdb-test 22 "y" "68" } } */
+    }
+  return x;
+}
+
+int (*p) (int, int) = fn2;
+
+int
+main ()
+{
+  __asm volatile ("" : : : "memory");
+  int (*q) (int, int) = p;
+  __asm volatile ("" : : : "memory");
+  q (6, 25);
+  q (4, 117);
+  q (0, 0);
+  return 0;
+}
--- gcc/testsuite/gcc.dg/guality/pr54519-5.c.jj	2012-08-09 23:41:08.146390865 +0200
+++ gcc/testsuite/gcc.dg/guality/pr54519-5.c	2012-09-10 18:30:07.683362384 +0200
@@ -0,0 +1,45 @@ 
+/* PR debug/54519 */
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+__attribute__((noinline, noclone)) void
+fn1 (int x)
+{
+  __asm volatile ("" : "+r" (x) : : "memory");
+}
+
+static int
+fn2 (int x, int y)
+{
+  if (y)
+    {
+      fn1 (x);		/* { dg-final { gdb-test 17 "x" "6" } } */
+      fn1 (x);		/* { dg-final { gdb-test 17 "y" "25" } } */
+      fn1 (x);
+      fn1 (x);
+      y = -2 + x;
+      y = y * y * y + y;
+      fn1 (x + y);	/* { dg-final { gdb-test 22 "y" "68" } } */
+    }
+  return x;
+}
+
+__attribute__((noinline, noclone)) int
+fn3 (int x, int y)
+{
+  return fn2 (x, y);
+}
+
+__attribute__((noinline, noclone)) int
+fn4 (int x, int y)
+{
+  return fn2 (x, y);
+}
+
+int
+main ()
+{
+  fn3 (6, 25);
+  fn4 (4, 117);
+  return 0;
+}