Patchwork [PR48866] three alternative fixes

login
register
mail settings
Submitter Alexandre Oliva
Date June 3, 2011, 1:16 a.m.
Message ID <or62onogs9.fsf@livre.localdomain>
Download mbox | patch
Permalink /patch/98503/
State New
Headers show

Comments

Alexandre Oliva - June 3, 2011, 1:16 a.m.
On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:

> On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
>> 1. emit debug temps for replaceable DEFs that end up being referenced in
>> debug insns.  We already have some code to try to deal with this, but it
>> emits the huge expressions we'd rather avoid, and it may create
>> unnecessary duplication.  This new approach emits a placeholder instead
>> of skipping replaceable DEFs altogether, and then, if the DEF is
>> referenced in a debug insn (perhaps during the late debug re-expasion of
>> some other placeholder), it is expanded.  Placeholders that end up not
>> being referenced are then throw away.

> This is my favorite option, for it's safest: it doesn't change
> executable code at all (or should I say it *shouldn't* change it, for I
> haven't verified that it doesn't), retaining any register pressure
> benefits from TER.

This revised and retested version records expansions in an array indexed
on SSA version rather than a pointer_map, as suggested by Matz.

Patch

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/48866
	* cfgexpand.c (def_expansions): New.
	(def_expansions_init): New.
	(def_expansions_remove_placeholder, def_expansions_fini): New.
	(def_get_expansion_ptr): New.
	(expand_debug_expr): Create debug temps as needed.
	(expand_debug_insn): New, split out of...
	(expand_debug_locations): ... this.
	(gen_emit_debug_insn): New, split out of...
	(expand_gimple_basic_block): ... this.  Simplify expansion of
	debug stmts.  Emit placeholders for replaceable DEFs, rather
	than debug temps at last non-debug uses.
	(gimple_expand_cfg): Initialize and finalize expansions cache.

Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig	2011-06-01 19:45:02.520428653 -0300
+++ gcc/cfgexpand.c	2011-06-01 20:20:02.014975168 -0300
@@ -2337,6 +2337,70 @@  convert_debug_memory_address (enum machi
   return x;
 }
 
+/* Mark debug insns that are placeholders for replaceable SSA_NAMEs
+   that have not been expanded yet.  */
+#define DEBUG_INSN_TOEXPAND(RTX)					\
+  (RTL_FLAG_CHECK1("DEBUG_INSN_TOEXPAND", (RTX), DEBUG_INSN)->used)
+
+/* Map replaceable SSA_NAMEs versions to their RTL expansions.  */
+static rtx *def_expansions;
+
+/* Initialize the def_expansions data structure.  This is to be called
+   before expansion of a function starts.  */
+
+static void
+def_expansions_init (void)
+{
+  gcc_checking_assert (!def_expansions);
+  def_expansions = XCNEWVEC (rtx, num_ssa_names);
+}
+
+/* Remove the DEBUG_INSN INSN if it still binds an SSA_NAME.  */
+
+static bool
+def_expansions_remove_placeholder (rtx insn)
+{
+  gcc_checking_assert (insn);
+
+  if (TREE_CODE (INSN_VAR_LOCATION_DECL (insn)) == SSA_NAME)
+    {
+      gcc_assert (!DEBUG_INSN_TOEXPAND (insn));
+      remove_insn (insn);
+    }
+
+  return true;
+}
+
+/* Finalize the def_expansions data structure.  This is to be called
+   at the end of the expansion of a function.  */
+
+static void
+def_expansions_fini (void)
+{
+  int i = num_ssa_names;
+
+  gcc_checking_assert (def_expansions);
+  while (i--)
+    if (def_expansions[i])
+      def_expansions_remove_placeholder (def_expansions[i]);
+  XDELETEVEC (def_expansions);
+  def_expansions = NULL;
+}
+
+/* Return a pointer to the rtx expanded from EXP.  EXP must be a
+   replaceable SSA_NAME.  */
+
+static rtx *
+def_get_expansion_ptr (tree exp)
+{
+  gcc_checking_assert (def_expansions);
+  gcc_checking_assert (TREE_CODE (exp) == SSA_NAME);
+  gcc_checking_assert (bitmap_bit_p (SA.values, SSA_NAME_VERSION (exp)));
+  return &def_expansions[SSA_NAME_VERSION (exp)];
+}
+
+static void expand_debug_insn (rtx insn);
+
 /* Return an RTX equivalent to the value of the tree expression
    EXP.  */
 
@@ -3131,7 +3195,30 @@  expand_debug_expr (tree exp)
 	gimple g = get_gimple_for_ssa_name (exp);
 	if (g)
 	  {
-	    op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g));
+	    rtx insn = *def_get_expansion_ptr (exp);
+	    tree vexpr;
+
+	    /* If this still has the original SSA_NAME, emit a debug
+	       temp and compute the RTX value.  */
+	    if (TREE_CODE (INSN_VAR_LOCATION_DECL (insn)) == SSA_NAME)
+	      {
+		tree var = SSA_NAME_VAR (INSN_VAR_LOCATION_DECL (insn));
+
+		vexpr = make_node (DEBUG_EXPR_DECL);
+		DECL_ARTIFICIAL (vexpr) = 1;
+		TREE_TYPE (vexpr) = TREE_TYPE (var);
+		DECL_MODE (vexpr) = DECL_MODE (var);
+		INSN_VAR_LOCATION_DECL (insn) = vexpr;
+
+		gcc_checking_assert (!DEBUG_INSN_TOEXPAND (insn));
+		DEBUG_INSN_TOEXPAND (insn) = 1;
+		expand_debug_insn (insn);
+	      }
+	    else
+	      vexpr = INSN_VAR_LOCATION_DECL (insn);
+
+	    op0 = expand_debug_expr (vexpr);
+
 	    if (!op0)
 	      return NULL;
 	  }
@@ -3293,6 +3380,45 @@  expand_debug_expr (tree exp)
     }
 }
 
+/* Expand the LOC value of the debug insn INSN.  */
+
+static void
+expand_debug_insn (rtx insn)
+{
+  tree value = (tree)INSN_VAR_LOCATION_LOC (insn);
+  rtx val;
+  enum machine_mode mode;
+
+  gcc_checking_assert (TREE_CODE (INSN_VAR_LOCATION_DECL (insn)) != SSA_NAME);
+  gcc_checking_assert (DEBUG_INSN_TOEXPAND (insn));
+  DEBUG_INSN_TOEXPAND (insn) = 0;
+
+  if (value == NULL_TREE)
+    val = NULL_RTX;
+  else
+    {
+      rtx last = get_last_insn ();
+      val = expand_debug_expr (value);
+      gcc_checking_assert (last == get_last_insn ());
+    }
+
+  if (!val)
+    val = gen_rtx_UNKNOWN_VAR_LOC ();
+  else
+    {
+      mode = GET_MODE (INSN_VAR_LOCATION (insn));
+
+      gcc_assert (mode == GET_MODE (val)
+		  || (GET_MODE (val) == VOIDmode
+		      && (CONST_INT_P (val)
+			  || GET_CODE (val) == CONST_FIXED
+			  || GET_CODE (val) == CONST_DOUBLE
+			  || GET_CODE (val) == LABEL_REF)));
+    }
+
+  INSN_VAR_LOCATION_LOC (insn) = val;
+}
+
 /* Expand the _LOCs in debug insns.  We run this after expanding all
    regular insns, so that any variables referenced in the function
    will have their DECL_RTLs set.  */
@@ -3301,7 +3427,6 @@  static void
 expand_debug_locations (void)
 {
   rtx insn;
-  rtx last = get_last_insn ();
   int save_strict_alias = flag_strict_aliasing;
 
   /* New alias sets while setting up memory attributes cause
@@ -3310,38 +3435,54 @@  expand_debug_locations (void)
   flag_strict_aliasing = 0;
 
   for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
-    if (DEBUG_INSN_P (insn))
-      {
-	tree value = (tree)INSN_VAR_LOCATION_LOC (insn);
-	rtx val;
-	enum machine_mode mode;
+    /* Skip debug insns that have SSA_NAMEs as bound decls.  These
+       will only have their values expanded if referenced by other
+       debug insns, and they are removed if not expanded.  */
+    if (DEBUG_INSN_P (insn) && DEBUG_INSN_TOEXPAND (insn))
+      expand_debug_insn (insn);
 
-	if (value == NULL_TREE)
-	  val = NULL_RTX;
-	else
-	  {
-	    val = expand_debug_expr (value);
-	    gcc_assert (last == get_last_insn ());
-	  }
+  flag_strict_aliasing = save_strict_alias;
+}
 
-	if (!val)
-	  val = gen_rtx_UNKNOWN_VAR_LOC ();
-	else
-	  {
-	    mode = GET_MODE (INSN_VAR_LOCATION (insn));
+/* Emit a debug insn that binds VAR to VALUE, with location and block
+   information taken from STMT.  */
 
-	    gcc_assert (mode == GET_MODE (val)
-			|| (GET_MODE (val) == VOIDmode
-			    && (CONST_INT_P (val)
-				|| GET_CODE (val) == CONST_FIXED
-				|| GET_CODE (val) == CONST_DOUBLE
-				|| GET_CODE (val) == LABEL_REF)));
-	  }
+static rtx
+gen_emit_debug_insn (tree var, tree value, gimple stmt)
+{
+  enum machine_mode mode;
+  location_t sloc = get_curr_insn_source_location ();
+  tree sblock = get_curr_insn_block ();
+  rtx val, insn;
+
+  if (DECL_P (var))
+    mode = DECL_MODE (var);
+  else if (TREE_CODE (var) == SSA_NAME)
+    mode = DECL_MODE (SSA_NAME_VAR (var));
+  else
+    mode = TYPE_MODE (TREE_TYPE (var));
 
-	INSN_VAR_LOCATION_LOC (insn) = val;
-      }
+  val = gen_rtx_VAR_LOCATION
+    (mode, var, (rtx)value, VAR_INIT_STATUS_INITIALIZED);
 
-  flag_strict_aliasing = save_strict_alias;
+  set_curr_insn_source_location (gimple_location (stmt));
+  set_curr_insn_block (gimple_block (stmt));
+  insn = emit_debug_insn (val);
+  set_curr_insn_source_location (sloc);
+  set_curr_insn_block (sblock);
+
+  DEBUG_INSN_TOEXPAND (insn) = (TREE_CODE (var) != SSA_NAME);
+
+  if (dump_file && (dump_flags & TDF_DETAILS))
+    {
+      /* We can't dump the insn with a TREE where an RTX
+	 is expected.  */
+      PAT_VAR_LOCATION_LOC (val) = const0_rtx;
+      maybe_dump_rtl_for_gimple_stmt (stmt, PREV_INSN (insn));
+      PAT_VAR_LOCATION_LOC (val) = (rtx)value;
+    }
+
+  return insn;
 }
 
 /* Expand basic block BB from GIMPLE trees to RTL.  */
@@ -3433,104 +3574,6 @@  expand_gimple_basic_block (basic_block b
 
       stmt = gsi_stmt (gsi);
 
-      /* If this statement is a non-debug one, and we generate debug
-	 insns, then this one might be the last real use of a TERed
-	 SSA_NAME, but where there are still some debug uses further
-	 down.  Expanding the current SSA name in such further debug
-	 uses by their RHS might lead to wrong debug info, as coalescing
-	 might make the operands of such RHS be placed into the same
-	 pseudo as something else.  Like so:
-	   a_1 = a_0 + 1;   // Assume a_1 is TERed and a_0 is dead
-	   use(a_1);
-	   a_2 = ...
-           #DEBUG ... => a_1
-	 As a_0 and a_2 don't overlap in lifetime, assume they are coalesced.
-	 If we now would expand a_1 by it's RHS (a_0 + 1) in the debug use,
-	 the write to a_2 would actually have clobbered the place which
-	 formerly held a_0.
-
-	 So, instead of that, we recognize the situation, and generate
-	 debug temporaries at the last real use of TERed SSA names:
-	   a_1 = a_0 + 1;
-           #DEBUG #D1 => a_1
-	   use(a_1);
-	   a_2 = ...
-           #DEBUG ... => #D1
-	 */
-      if (MAY_HAVE_DEBUG_INSNS
-	  && SA.values
-	  && !is_gimple_debug (stmt))
-	{
-	  ssa_op_iter iter;
-	  tree op;
-	  gimple def;
-
-	  location_t sloc = get_curr_insn_source_location ();
-	  tree sblock = get_curr_insn_block ();
-
-	  /* Look for SSA names that have their last use here (TERed
-	     names always have only one real use).  */
-	  FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_USE)
-	    if ((def = get_gimple_for_ssa_name (op)))
-	      {
-		imm_use_iterator imm_iter;
-		use_operand_p use_p;
-		bool have_debug_uses = false;
-
-		FOR_EACH_IMM_USE_FAST (use_p, imm_iter, op)
-		  {
-		    if (gimple_debug_bind_p (USE_STMT (use_p)))
-		      {
-			have_debug_uses = true;
-			break;
-		      }
-		  }
-
-		if (have_debug_uses)
-		  {
-		    /* OP is a TERed SSA name, with DEF it's defining
-		       statement, and where OP is used in further debug
-		       instructions.  Generate a debug temporary, and
-		       replace all uses of OP in debug insns with that
-		       temporary.  */
-		    gimple debugstmt;
-		    tree value = gimple_assign_rhs_to_tree (def);
-		    tree vexpr = make_node (DEBUG_EXPR_DECL);
-		    rtx val;
-		    enum machine_mode mode;
-
-		    set_curr_insn_source_location (gimple_location (def));
-		    set_curr_insn_block (gimple_block (def));
-
-		    DECL_ARTIFICIAL (vexpr) = 1;
-		    TREE_TYPE (vexpr) = TREE_TYPE (value);
-		    if (DECL_P (value))
-		      mode = DECL_MODE (value);
-		    else
-		      mode = TYPE_MODE (TREE_TYPE (value));
-		    DECL_MODE (vexpr) = mode;
-
-		    val = gen_rtx_VAR_LOCATION
-			(mode, vexpr, (rtx)value, VAR_INIT_STATUS_INITIALIZED);
-
-		    emit_debug_insn (val);
-
-		    FOR_EACH_IMM_USE_STMT (debugstmt, imm_iter, op)
-		      {
-			if (!gimple_debug_bind_p (debugstmt))
-			  continue;
-
-			FOR_EACH_IMM_USE_ON_STMT (use_p, imm_iter)
-			  SET_USE (use_p, vexpr);
-
-			update_stmt (debugstmt);
-		      }
-		  }
-	      }
-	  set_curr_insn_source_location (sloc);
-	  set_curr_insn_block (sblock);
-	}
-
       currently_expanding_gimple_stmt = stmt;
 
       /* Expand this statement, then evaluate the resulting RTL and
@@ -3543,64 +3586,15 @@  expand_gimple_basic_block (basic_block b
 	}
       else if (gimple_debug_bind_p (stmt))
 	{
-	  location_t sloc = get_curr_insn_source_location ();
-	  tree sblock = get_curr_insn_block ();
-	  gimple_stmt_iterator nsi = gsi;
-
-	  for (;;)
-	    {
-	      tree var = gimple_debug_bind_get_var (stmt);
-	      tree value;
-	      rtx val;
-	      enum machine_mode mode;
-
-	      if (gimple_debug_bind_has_value_p (stmt))
-		value = gimple_debug_bind_get_value (stmt);
-	      else
-		value = NULL_TREE;
-
-	      last = get_last_insn ();
+	  tree var = gimple_debug_bind_get_var (stmt);
+	  tree value;
 
-	      set_curr_insn_source_location (gimple_location (stmt));
-	      set_curr_insn_block (gimple_block (stmt));
-
-	      if (DECL_P (var))
-		mode = DECL_MODE (var);
-	      else
-		mode = TYPE_MODE (TREE_TYPE (var));
-
-	      val = gen_rtx_VAR_LOCATION
-		(mode, var, (rtx)value, VAR_INIT_STATUS_INITIALIZED);
-
-	      emit_debug_insn (val);
-
-	      if (dump_file && (dump_flags & TDF_DETAILS))
-		{
-		  /* We can't dump the insn with a TREE where an RTX
-		     is expected.  */
-		  PAT_VAR_LOCATION_LOC (val) = const0_rtx;
-		  maybe_dump_rtl_for_gimple_stmt (stmt, last);
-		  PAT_VAR_LOCATION_LOC (val) = (rtx)value;
-		}
-
-	      /* In order not to generate too many debug temporaries,
-	         we delink all uses of debug statements we already expanded.
-		 Therefore debug statements between definition and real
-		 use of TERed SSA names will continue to use the SSA name,
-		 and not be replaced with debug temps.  */
-	      delink_stmt_imm_use (stmt);
-
-	      gsi = nsi;
-	      gsi_next (&nsi);
-	      if (gsi_end_p (nsi))
-		break;
-	      stmt = gsi_stmt (nsi);
-	      if (!gimple_debug_bind_p (stmt))
-		break;
-	    }
+	  if (gimple_debug_bind_has_value_p (stmt))
+	    value = gimple_debug_bind_get_value (stmt);
+	  else
+	    value = NULL_TREE;
 
-	  set_curr_insn_source_location (sloc);
-	  set_curr_insn_block (sblock);
+	  gen_emit_debug_insn (var, value, stmt);
 	}
       else
 	{
@@ -3621,14 +3615,29 @@  expand_gimple_basic_block (basic_block b
 	      def_operand_p def_p;
 	      def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
 
-	      if (def_p != NULL)
+	      /* If this stmt is in the list of replaceable
+		 expressions, expand a placeholder for a debug insn.  */
+	      if (def_p != NULL && SA.values
+		  && bitmap_bit_p (SA.values,
+				   SSA_NAME_VERSION (DEF_FROM_PTR (def_p))))
 		{
-		  /* Ignore this stmt if it is in the list of
-		     replaceable expressions.  */
-		  if (SA.values
-		      && bitmap_bit_p (SA.values,
-				       SSA_NAME_VERSION (DEF_FROM_PTR (def_p))))
+		  tree def = DEF_FROM_PTR (def_p);
+		  rtx *xp;
+
+		  gcc_checking_assert (TREE_CODE (def) == SSA_NAME
+				       && DECL_P (SSA_NAME_VAR (def)));
+
+		  if (!MAY_HAVE_DEBUG_INSNS)
 		    continue;
+
+		  xp = def_get_expansion_ptr (def);
+		  gcc_checking_assert (!*xp);
+		  *xp = last
+		    = gen_emit_debug_insn (def,
+					   gimple_assign_rhs_to_tree (stmt),
+					   stmt);
+
+		  continue;
 		}
 	      last = expand_gimple_stmt (stmt);
 	      maybe_dump_rtl_for_gimple_stmt (stmt, last);
@@ -3970,6 +3979,7 @@  gimple_expand_cfg (void)
   rtx var_seq;
   unsigned i;
 
+
   timevar_push (TV_OUT_OF_SSA);
   rewrite_out_of_ssa (&SA);
   timevar_pop (TV_OUT_OF_SSA);
@@ -4112,11 +4122,18 @@  gimple_expand_cfg (void)
     e->flags &= ~EDGE_EXECUTABLE;
 
   lab_rtx_for_bb = pointer_map_create ();
+
+  if (MAY_HAVE_DEBUG_INSNS)
+    def_expansions_init ();
+
   FOR_BB_BETWEEN (bb, init_block->next_bb, EXIT_BLOCK_PTR, next_bb)
     bb = expand_gimple_basic_block (bb);
 
   if (MAY_HAVE_DEBUG_INSNS)
-    expand_debug_locations ();
+    {
+      expand_debug_locations ();
+      def_expansions_fini ();
+    }
 
   execute_free_datastructures ();
   timevar_push (TV_OUT_OF_SSA);