diff mbox series

[2/3] Track debug locations of some memory variables

Message ID mptftottia8.fsf@arm.com
State New
Headers show
Series Improve debug info for addressable vars | expand

Commit Message

Richard Sandiford June 1, 2019, 3:52 p.m. UTC
This patch tries to track the debug location of memory variables
that have a target_for_debug_bind (and thus have a type that
satisfies is_gimple_reg_type).  For each such variable V:

* "# DEBUG V s=> V" marks the start of V's lifetime

* during V's lifetime, "# DEBUG V => X" records that the value of V is
  given by X instead of DECL_RTL (V).

* each assignment to V implicitly reestablishes the link between V
  and DECL_RTL (V).

* as before, "V = {CLOBBER}" marks the end of V's lifetime.

When removing "V = X", gsi_remove tries to insert the equivalent of
"# DEBUG V => X" at the place of the original assignment, falling back
on a "# DEBUG V => NULL" reset if that fails.

The change to insert_debug_temp_for_var_def is mostly reindentation.

The patch only handles VAR_DECLs, but we might want to extend it
to PARM_DECLs too.


2019-06-01  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* tree-core.h (tree_decl_with_vis): Group bitfields into bytes.
	(tree_decl_with_vis::track_direct_refs_for_debug_p): New field.
	* tree-streamer-in.c (unpack_ts_decl_with_vis_value_fields): Handle it.
	* tree-streamer-out.c (pack_ts_decl_with_vis_value_fields): Likewise.
	* tree.h (DECL_TRACK_DIRECT_REFS_FOR_DEBUG_P): New macro.
	(track_direct_refs_for_debug_p): New function.
	* cfgexpand.c (expand_debug_locations): Use DECL_RTL_REF to
	handle VAR s=> VAR.
	(expand_gimple_basic_block): Insert the initialized equivalent
	of VAR s=> VAR after each direct assignment to VAR.
	* gimplify.c (scoped_possible_mem_var_p): New function, split out
	from...
	(gimplify_bind_expr): ...here.  Decide whether it would be worth
	tracking the location of a non-gimple-reg VAR_DECL using debug
	stmts and insns.  Mark them with DECL_TRACK_DIRECT_REFS_FOR_DEBUG_P
	if so and use gimple_debug_source_binds to mark the start of their
	lifetime.
	* gimple-low.c (lower_stmt): Allow gimple_debug_source_binds
	before lowering.
	* tree-cfg.c (make_blocks): Handle gimple_debug_source_binds.
	* tree-ssa.c (insert_debug_temp_for_var_def): Handle VAR_DECLs
	as well as SSA_NAMEs.
	(insert_debug_temps_for_defs): Insert gimple_debug_binds for
	VAR_DECLs for which DECL_TRACK_DIRECT_REFS_FOR_DEBUG_P is set.

gcc/testsuite/
	* gcc.dg/guality/addr-taken-1.c: New test.
	* gcc.dg/guality/addr-taken-2.c: Likewise.
diff mbox series

Patch

Index: gcc/tree-core.h
===================================================================
--- gcc/tree-core.h	2019-05-29 10:49:39.896700883 +0100
+++ gcc/tree-core.h	2019-06-01 16:38:35.421667415 +0100
@@ -1761,13 +1761,12 @@  struct GTY(()) tree_decl_with_vis {
  unsigned dllimport_flag : 1;
  /* Don't belong to VAR_DECL exclusively.  */
  unsigned weak_flag : 1;
-
  unsigned seen_in_bind_expr : 1;
+
  unsigned comdat_flag : 1;
  /* Used for FUNCTION_DECL, VAR_DECL and in C++ for TYPE_DECL.  */
  ENUM_BITFIELD(symbol_visibility) visibility : 2;
  unsigned visibility_specified : 1;
-
  /* Belong to FUNCTION_DECL exclusively.  */
  unsigned init_priority_p : 1;
  /* Used by C++ only.  Might become a generic decl flag.  */
@@ -1776,11 +1775,16 @@  struct GTY(()) tree_decl_with_vis {
  unsigned cxx_constructor : 1;
  /* Belong to FUNCTION_DECL exclusively.  */
  unsigned cxx_destructor : 1;
+
  /* Belong to FUNCTION_DECL exclusively.  */
  unsigned final : 1;
  /* Belong to FUNCTION_DECL exclusively.  */
  unsigned regdecl_flag : 1;
- /* 14 unused bits. */
+ /* Records whether direct references to VAR_DECLs should be tracked via
+    debug stmts.  */
+ unsigned track_direct_refs_for_debug_p : 1;
+
+ /* 13 unused bits. */
 };
 
 struct GTY(()) tree_var_decl {
Index: gcc/tree-streamer-in.c
===================================================================
--- gcc/tree-streamer-in.c	2019-05-29 10:49:39.896700883 +0100
+++ gcc/tree-streamer-in.c	2019-06-01 16:38:35.421667415 +0100
@@ -307,6 +307,8 @@  unpack_ts_decl_with_vis_value_fields (st
     {
       DECL_HARD_REGISTER (expr) = (unsigned) bp_unpack_value (bp, 1);
       DECL_IN_CONSTANT_POOL (expr) = (unsigned) bp_unpack_value (bp, 1);
+      DECL_TRACK_DIRECT_REFS_FOR_DEBUG_P (expr)
+	= (unsigned) bp_unpack_value (bp, 1);
     }
 
   if (TREE_CODE (expr) == FUNCTION_DECL)
Index: gcc/tree-streamer-out.c
===================================================================
--- gcc/tree-streamer-out.c	2019-05-29 10:49:39.620701683 +0100
+++ gcc/tree-streamer-out.c	2019-06-01 16:38:35.421667415 +0100
@@ -269,6 +269,7 @@  pack_ts_decl_with_vis_value_fields (stru
       bp_pack_value (bp, DECL_HARD_REGISTER (expr), 1);
       /* DECL_IN_TEXT_SECTION is set during final asm output only. */
       bp_pack_value (bp, DECL_IN_CONSTANT_POOL (expr), 1);
+      bp_pack_value (bp, DECL_TRACK_DIRECT_REFS_FOR_DEBUG_P (expr), 1);
     }
 
   if (TREE_CODE (expr) == FUNCTION_DECL)
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	2019-05-29 10:49:37.876706747 +0100
+++ gcc/tree.h	2019-06-01 16:38:35.421667415 +0100
@@ -2888,6 +2888,12 @@  #define DECL_DEBUG_EXPR(NODE) \
 #define SET_DECL_DEBUG_EXPR(NODE, VAL) \
   (decl_debug_expr_insert (VAR_DECL_CHECK (NODE), VAL))
 
+/* For a VAR_DECL, true if accesses to the variable can't be rewritten
+   into SSA form but if we nevertheless want to use debug stmts
+   (and later debug insns) to track its location.  */
+#define DECL_TRACK_DIRECT_REFS_FOR_DEBUG_P(NODE) \
+  (VAR_DECL_CHECK (NODE)->decl_with_vis.track_direct_refs_for_debug_p)
+
 extern priority_type decl_init_priority_lookup (tree);
 extern priority_type decl_fini_priority_lookup (tree);
 extern void decl_init_priority_insert (tree, priority_type);
@@ -4733,6 +4739,15 @@  extern tree get_unwidened (tree, tree);
 
 extern tree get_narrower (tree, int *);
 
+/* Return true if T is a DECL for which DECL_TRACK_DIRECT_REFS_FOR_DEBUG_P
+   applies.  */
+
+inline bool
+track_direct_refs_for_debug_p (const_tree t)
+{
+  return VAR_P (t) && DECL_TRACK_DIRECT_REFS_FOR_DEBUG_P (t);
+}
+
 /* Return true if T is an expression that get_inner_reference handles.  */
 
 static inline bool
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	2019-05-29 10:49:39.524701962 +0100
+++ gcc/cfgexpand.c	2019-06-01 16:38:35.413667438 +0100
@@ -5429,8 +5429,15 @@  expand_debug_locations (void)
 	  val = NULL_RTX;
 	else
 	  {
-	    if (INSN_VAR_LOCATION_STATUS (insn)
-		== VAR_INIT_STATUS_UNINITIALIZED)
+	    if (INSN_VAR_LOCATION_DECL (insn) == value)
+	      {
+		if (DECL_RTL_SET_P (value))
+		  val = gen_rtx_DECL_RTL_REF (DECL_MODE (value), value);
+		else
+		  val = NULL_RTX;
+	      }
+	    else if (INSN_VAR_LOCATION_STATUS (insn)
+		     == VAR_INIT_STATUS_UNINITIALIZED)
 	      val = expand_debug_source_expr (value);
 	    /* The avoid_deep_ter_for_debug function inserts
 	       debug bind stmts after SSA_NAME definition, with the
@@ -5900,6 +5907,19 @@  expand_gimple_basic_block (basic_block b
 	      maybe_dump_rtl_for_gimple_stmt (stmt, last);
 	    }
 	}
+      if (MAY_HAVE_DEBUG_BIND_INSNS)
+	{
+	  /* If the statement is assigning to a memory variable whose
+	     location we're tracking for debug purposes, emit a note
+	     to say that the variable now lives in memory.  */
+	  tree lhs = gimple_get_lhs (stmt);
+	  if (lhs && track_direct_refs_for_debug_p (lhs))
+	    {
+	      rtx val = gen_rtx_VAR_LOCATION (DECL_MODE (lhs), lhs, (rtx) lhs,
+					      VAR_INIT_STATUS_INITIALIZED);
+	      emit_debug_insn (val);
+	    }
+	}
     }
 
   currently_expanding_gimple_stmt = NULL;
Index: gcc/gimplify.c
===================================================================
--- gcc/gimplify.c	2019-05-31 17:26:54.647201231 +0100
+++ gcc/gimplify.c	2019-06-01 16:38:35.417667427 +0100
@@ -1295,6 +1295,21 @@  asan_poison_variables (hash_set<tree> *v
     }
 }
 
+/* Return true if variable VAR is local to its BIND_EXPR and might need
+   to live in memory.  */
+
+static bool
+scoped_possible_mem_var_p (tree var)
+{
+  return (!DECL_HARD_REGISTER (var)
+	  && !TREE_THIS_VOLATILE (var)
+	  && !DECL_HAS_VALUE_EXPR_P (var)
+	  /* Only care about variables that have to be in memory.  Others
+	     will be rewritten into SSA names, hence moved to the
+	     top-level.  */
+	  && !is_gimple_reg (var));
+}
+
 /* Gimplify a BIND_EXPR.  Just voidify and recurse.  */
 
 static enum gimplify_status
@@ -1313,6 +1328,7 @@  gimplify_bind_expr (tree *expr_p, gimple
   tree temp = voidify_wrapper_expr (bind_expr, NULL);
 
   /* Mark variables seen in this bind expr.  */
+  body = NULL;
   for (t = BIND_EXPR_VARS (bind_expr); t ; t = DECL_CHAIN (t))
     {
       if (VAR_P (t))
@@ -1348,6 +1364,18 @@  gimplify_bind_expr (tree *expr_p, gimple
 	  && (VAR_P (t) && !DECL_HARD_REGISTER (t))
 	  && !needs_to_live_in_memory (t))
 	DECL_GIMPLE_REG_P (t) = 1;
+
+      if (MAY_HAVE_DEBUG_BIND_INSNS
+	  && VAR_P (t)
+	  && !is_global_var (t)
+	  && DECL_CONTEXT (t) == current_function_decl
+	  && scoped_possible_mem_var_p (t)
+	  && target_for_debug_bind (t) == t)
+	{
+	  gdebug *sb = gimple_build_debug_source_bind (t, t, NULL);
+	  gimple_seq_add_stmt (&body, sb);
+	  DECL_TRACK_DIRECT_REFS_FOR_DEBUG_P (t) = 1;
+	}
     }
 
   bind_stmt = gimple_build_bind (BIND_EXPR_VARS (bind_expr), NULL,
@@ -1358,7 +1386,6 @@  gimplify_bind_expr (tree *expr_p, gimple
   gimplify_ctxp->save_stack = false;
 
   /* Gimplify the body into the GIMPLE_BIND tuple's body.  */
-  body = NULL;
   gimplify_stmt (&BIND_EXPR_BODY (bind_expr), &body);
   gimple_bind_set_body (bind_stmt, body);
 
@@ -1401,13 +1428,7 @@  gimplify_bind_expr (tree *expr_p, gimple
 	  && !is_global_var (t)
 	  && DECL_CONTEXT (t) == current_function_decl)
 	{
-	  if (!DECL_HARD_REGISTER (t)
-	      && !TREE_THIS_VOLATILE (t)
-	      && !DECL_HAS_VALUE_EXPR_P (t)
-	      /* Only care for variables that have to be in memory.  Others
-		 will be rewritten into SSA names, hence moved to the
-		 top-level.  */
-	      && !is_gimple_reg (t)
+	  if (scoped_possible_mem_var_p (t)
 	      && flag_stack_reuse != SR_NONE)
 	    {
 	      tree clobber = build_clobber (TREE_TYPE (t));
Index: gcc/gimple-low.c
===================================================================
--- gcc/gimple-low.c	2019-04-04 08:34:52.221937259 +0100
+++ gcc/gimple-low.c	2019-06-01 16:38:35.417667427 +0100
@@ -309,10 +309,12 @@  lower_stmt (gimple_stmt_iterator *gsi, s
       break;
 
     case GIMPLE_DEBUG:
-      gcc_checking_assert (cfun->debug_nonbind_markers);
-      /* We can't possibly have debug bind stmts before lowering, we
-	 first emit them when entering SSA.  */
-      gcc_checking_assert (gimple_debug_nonbind_marker_p (stmt));
+      /* The only debug bind stmts we create before lowering are
+	 source binds of the form "x s=> x", to mark the start of
+	 a variable's lifetime.  */
+      gcc_checking_assert ((cfun->debug_nonbind_markers
+			    && gimple_debug_nonbind_marker_p (stmt))
+			   || gimple_debug_source_bind_p (stmt));
       /* Propagate fallthruness.  */
       /* If the function (e.g. from PCH) had debug stmts, but they're
 	 disabled for this compilation, remove them.  */
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c	2019-05-29 10:49:39.900700870 +0100
+++ gcc/tree-cfg.c	2019-06-01 16:38:35.421667415 +0100
@@ -593,8 +593,8 @@  make_blocks_1 (gimple_seq seq, basic_blo
 static void
 make_blocks (gimple_seq seq)
 {
-  /* Look for debug markers right before labels, and move the debug
-     stmts after the labels.  Accepting labels among debug markers
+  /* Look for debug stmts right before labels, and move the debug
+     stmts after the labels.  Accepting labels among debug stmts
      adds no value, just complexity; if we wanted to annotate labels
      with view numbers (so sequencing among markers would matter) or
      somesuch, we're probably better off still moving the labels, but
@@ -635,7 +635,11 @@  make_blocks (gimple_seq seq)
 	  /* Move the debug stmt at I after LABEL.  */
 	  if (is_gimple_debug (stmt))
 	    {
-	      gcc_assert (gimple_debug_nonbind_marker_p (stmt));
+	      /* Handle both debug markers and source binds of the form
+		 VAR s=> VAR (which denote the beginning of VAR's
+		 lifetime).  */
+	      gcc_assert (gimple_debug_nonbind_marker_p (stmt)
+			  || gimple_debug_source_bind_p (stmt));
 	      /* As STMT is removed, I advances to the stmt after
 		 STMT, so the gsi_prev in the for "increment"
 		 expression gets us to the stmt we're to visit after
Index: gcc/tree-ssa.c
===================================================================
--- gcc/tree-ssa.c	2019-05-29 10:49:39.892700893 +0100
+++ gcc/tree-ssa.c	2019-06-01 16:38:35.421667415 +0100
@@ -293,9 +293,13 @@  find_released_ssa_name (tree *tp, int *w
   return NULL_TREE;
 }
 
-/* Insert a DEBUG BIND stmt before the DEF of VAR if VAR is referenced
-   by other DEBUG stmts, and replace uses of the DEF with the
-   newly-created debug temp.  */
+/* Insert a DEBUG BIND stmt before the DEF of VAR in cases where either:
+
+   - VAR is a decl that satisfies track_direct_refs_for_debug_p
+   - VAR is an SSA_NAME that is referenced by other DEBUG stmts
+
+   In the latter case, replace uses of the DEF with the newly-created
+   debug temp.  */
 
 void
 insert_debug_temp_for_var_def (gimple_stmt_iterator *gsi, tree var)
@@ -310,35 +314,38 @@  insert_debug_temp_for_var_def (gimple_st
   if (!MAY_HAVE_DEBUG_BIND_STMTS)
     return;
 
-  /* If this name has already been registered for replacement, do nothing
-     as anything that uses this name isn't in SSA form.  */
-  if (name_registered_for_update_p (var))
-    return;
-
-  /* Check whether there are debug stmts that reference this variable and,
-     if there are, decide whether we should use a debug temp.  */
-  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, var)
+  if (TREE_CODE (var) == SSA_NAME)
     {
-      stmt = USE_STMT (use_p);
+      /* If this name has already been registered for replacement, do nothing
+	 as anything that uses this name isn't in SSA form.  */
+      if (name_registered_for_update_p (var))
+	return;
+
+      /* Check whether there are debug stmts that reference this variable and,
+	 if there are, decide whether we should use a debug temp.  */
+      FOR_EACH_IMM_USE_FAST (use_p, imm_iter, var)
+	{
+	  stmt = USE_STMT (use_p);
 
-      if (!gimple_debug_bind_p (stmt))
-	continue;
+	  if (!gimple_debug_bind_p (stmt))
+	    continue;
 
-      if (usecount++)
-	break;
+	  if (usecount++)
+	    break;
 
-      if (gimple_debug_bind_get_value (stmt) != var)
-	{
-	  /* Count this as an additional use, so as to make sure we
-	     use a temp unless VAR's definition has a SINGLE_RHS that
-	     can be shared.  */
-	  usecount++;
-	  break;
+	  if (gimple_debug_bind_get_value (stmt) != var)
+	    {
+	      /* Count this as an additional use, so as to make sure we
+		 use a temp unless VAR's definition has a SINGLE_RHS that
+		 can be shared.  */
+	      usecount++;
+	      break;
+	    }
 	}
-    }
 
-  if (!usecount)
-    return;
+      if (!usecount)
+	return;
+    }
 
   if (gsi)
     def_stmt = gsi_stmt (*gsi);
@@ -459,32 +466,38 @@  insert_debug_temp_for_var_def (gimple_st
 	}
     }
 
-  FOR_EACH_IMM_USE_STMT (stmt, imm_iter, var)
-    {
-      if (!gimple_debug_bind_p (stmt))
-	continue;
-
-      if (value)
-	{
-	  FOR_EACH_IMM_USE_ON_STMT (use_p, imm_iter)
-	    /* unshare_expr is not needed here.  vexpr is either a
-	       SINGLE_RHS, that can be safely shared, some other RHS
-	       that was unshared when we found it had a single debug
-	       use, or a DEBUG_EXPR_DECL, that can be safely
-	       shared.  */
-	    SET_USE (use_p, unshare_expr (value));
-	  /* If we didn't replace uses with a debug decl fold the
-	     resulting expression.  Otherwise we end up with invalid IL.  */
-	  if (TREE_CODE (value) != DEBUG_EXPR_DECL)
-	    {
-	      gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
-	      fold_stmt_inplace (&gsi);
-	    }
-	}
-      else
-	gimple_debug_bind_reset_value (stmt);
+  if (TREE_CODE (var) == SSA_NAME)
+    FOR_EACH_IMM_USE_STMT (stmt, imm_iter, var)
+      {
+	if (!gimple_debug_bind_p (stmt))
+	  continue;
+
+	if (value)
+	  {
+	    FOR_EACH_IMM_USE_ON_STMT (use_p, imm_iter)
+	      /* unshare_expr is not needed here.  vexpr is either a
+		 SINGLE_RHS, that can be safely shared, some other RHS
+		 that was unshared when we found it had a single debug
+		 use, or a DEBUG_EXPR_DECL, that can be safely
+		 shared.  */
+	      SET_USE (use_p, unshare_expr (value));
+	    /* If we didn't replace uses with a debug decl fold the
+	       resulting expression.  Otherwise we end up with invalid IL.  */
+	    if (TREE_CODE (value) != DEBUG_EXPR_DECL)
+	      {
+		gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+		fold_stmt_inplace (&gsi);
+	      }
+	  }
+	else
+	  gimple_debug_bind_reset_value (stmt);
 
-      update_stmt (stmt);
+	update_stmt (stmt);
+      }
+  else
+    {
+      gdebug *bind = gimple_build_debug_bind (var, value, def_stmt);
+      gsi_insert_before (gsi, bind, GSI_SAME_STMT);
     }
 }
 
@@ -514,6 +527,10 @@  insert_debug_temps_for_defs (gimple_stmt
 
       insert_debug_temp_for_var_def (gsi, var);
     }
+
+  tree lhs = gimple_get_lhs (stmt);
+  if (lhs && track_direct_refs_for_debug_p (lhs))
+    insert_debug_temp_for_var_def (gsi, lhs);
 }
 
 /* Reset all debug stmts that use SSA_NAME(s) defined in STMT.  */
Index: gcc/testsuite/gcc.dg/guality/addr-taken-1.c
===================================================================
--- /dev/null	2019-03-08 11:40:14.606883727 +0000
+++ gcc/testsuite/gcc.dg/guality/addr-taken-1.c	2019-06-01 16:38:35.417667427 +0100
@@ -0,0 +1,36 @@ 
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+void __attribute__((noipa))
+get_start (int *x)
+{
+  *x = 42;
+}
+
+void __attribute__((noipa))
+consume (int *x)
+{
+  *x += 1;
+}
+
+void __attribute__((noipa))
+test (int *x, int i1, int i2, int i3, int i4)
+{
+  int base;
+  get_start (&base);
+  x[i1] = base; /* { dg-final { gdb-test . "base" "42" } } */
+  base = 100;
+  x[i2] = base; /* { dg-final { gdb-test . "base" "100" } } */
+  base += 1;
+  x[i3] = base; /* { dg-final { gdb-test . "base" "101" } } */
+  base += 1;
+  consume (&base); /* { dg-final { gdb-test . "base" "102" } } */
+  x[i4] = base; /* { dg-final { gdb-test . "base" "103" } } */
+}
+
+int
+main (void)
+{
+  int x[4];
+  test (x, 0, 1, 2, 3);
+}
Index: gcc/testsuite/gcc.dg/guality/addr-taken-2.c
===================================================================
--- /dev/null	2019-03-08 11:40:14.606883727 +0000
+++ gcc/testsuite/gcc.dg/guality/addr-taken-2.c	2019-06-01 16:38:35.417667427 +0100
@@ -0,0 +1,34 @@ 
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+void __attribute__((noipa))
+get_start (int *x)
+{
+  *x = 42;
+}
+
+void __attribute__((noipa))
+consume (int *x)
+{
+  *x += 1;
+}
+
+void __attribute__((noipa))
+test (int *x, int i1, int i2, int i3)
+{
+  int base;
+  get_start (&base);
+  x[i1] = base; /* { dg-final { gdb-test . "base" "42" } } */
+  base += 1;
+  x[i2] = base; /* { dg-final { gdb-test . "base" "43" } } */
+  base += 1;
+  consume (&base); /* { dg-final { gdb-test . "base" "44" } } */
+  x[i3] = base; /* { dg-final { gdb-test . "base" "45" } } */
+}
+
+int
+main (void)
+{
+  int x[3];
+  test (x, 0, 1, 2);
+}