diff mbox

Don't reset debug stmts in delete_trivially_dead_insns unnecessarily

Message ID 20101005204850.GC4127@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Oct. 5, 2010, 8:48 p.m. UTC 
Hi!

This patch implements something similar to what we do in DF to avoid
resetting debug stmts, in particular if possible put a D#N temporary
right before the to be deleted trivially dead insn and use the D#N
DEBUG_EXPR in following DEBUG_INSNs instead of resetting them.

Bootstrapped/regtested on x86_64-linux and i686-linux.  Ok for trunk?

In i686-linux bootstrap/regtest delete_trivially_dead_insns had
to reset 18291 DEBUG_INSNs, but additional 19170 could be kept because of
this patch, on x86_64-linux 20807 resets happened and 3503 could be kept.
Still, even on x86_64-linux 180 DIEs in cc1plus got newly DW_AT_location
which they didn't have before.

2010-10-05  Jakub Jelinek  <jakub@redhat.com>

	* cse.c (is_dead_reg): Change into inline function that is not
	called through for_each_rtx.
	(set_live_p): Adjust caller.
	(insn_live_p): Don't reset DEBUG_INSNs here.
	(struct dead_debug_insn_data): New data. 
	(count_stores, is_dead_debug_insn, replace_dead_reg): New functions.
	(delete_trivially_dead_insns): If there is just one setter for the
	dead reg that is referenced by some DEBUG_INSNs, create a DEBUG_EXPR
	and add DEBUG_INSN for it right before the removed setter and
	use the DEBUG_EXPR instead of the dead pseudo.


	Jakub

Comments

Eric Botcazou Oct. 13, 2010, 3:56 p.m. UTC | #1 
> 2010-10-05  Jakub Jelinek  <jakub@redhat.com>
>
> 	* cse.c (is_dead_reg): Change into inline function that is not
> 	called through for_each_rtx.
> 	(set_live_p): Adjust caller.
> 	(insn_live_p): Don't reset DEBUG_INSNs here.
> 	(struct dead_debug_insn_data): New data.
> 	(count_stores, is_dead_debug_insn, replace_dead_reg): New functions.
> 	(delete_trivially_dead_insns): If there is just one setter for the
> 	dead reg that is referenced by some DEBUG_INSNs, create a DEBUG_EXPR
> 	and add DEBUG_INSN for it right before the removed setter and
> 	use the DEBUG_EXPR instead of the dead pseudo.

OK modulo the following nits:

>  /* Return true if a register is dead.  Can be used in for_each_rtx.  */
>
> -static int
> -is_dead_reg (rtx *loc, void *data)
> +static inline int
> +is_dead_reg (rtx x, int *counts)

Outdated head comment, and it would be nice to mention X in it now.


> +/* Count the number of stores into pseudo.  */
> +
> +static void
> +count_stores (rtx x, const_rtx set ATTRIBUTE_UNUSED, void *data)
> +{
> +  int *counts = (int *) data;
> +  if (REG_P (x) && REGNO (x) >= FIRST_PSEUDO_REGISTER)
> +    counts[REGNO (x)]++;
> +}

Mention that it's a callback for note_stores (or document the parameters).


> +/* Return if a DEBUG_INSN needs to be reset because some dead
> +   pseudo doesn't have a replacement.  */
> +
> +static int
> +is_dead_debug_insn (rtx *loc, void *data)
> +{
> +  rtx x = *loc;
> +  struct dead_debug_insn_data *ddid = (struct dead_debug_insn_data *)
> data; +
> +  if (is_dead_reg (x, ddid->counts))
> +    {
> +      if (ddid->replacements && ddid->replacements[REGNO (x)] != NULL_RTX)
> +	ddid->seen_repl = true;
> +      else
> +	return 1;
> +    }
> +  return 0;
> +}

Mention that it's a callback for for_each_rtx.


> +/* Replace a dead pseudo in a DEBUG_INSN with replacement DEBUG_EXPR.  */
> +
> +static rtx
> +replace_dead_reg (rtx x, const_rtx old_rtx ATTRIBUTE_UNUSED, void *data)
> +{
> +  rtx *replacements = (rtx *) data;
> +
> +  if (REG_P (x)
> +      && REGNO (x) >= FIRST_PSEUDO_REGISTER
> +      && replacements[REGNO (x)] != NULL_RTX)
> +    {
> +      if (GET_MODE (x) == GET_MODE (replacements[REGNO (x)]))
> +	return replacements[REGNO (x)];
> +      return lowpart_subreg (GET_MODE (x), replacements[REGNO (x)],
> +			     GET_MODE (replacements[REGNO (x)]));
> +    }
> +  return NULL_RTX;
> +}

Mention that it's a callback for simplify_replace_fn_rtx.


> +  if (MAY_HAVE_DEBUG_INSNS)
> +    {
> +      counts = XCNEWVEC (int, nreg * 3);
> +      for (insn = insns; insn; insn = NEXT_INSN (insn))
> +	if (DEBUG_INSN_P (insn))
> +	  count_reg_usage (INSN_VAR_LOCATION_LOC (insn), counts + nreg,
> +			   NULL_RTX, 1);
> +	else if (INSN_P (insn))
> +	  {
> +	    count_reg_usage (insn, counts, NULL_RTX, 1);
> +	    note_stores (PATTERN (insn), count_stores, counts + nreg * 2);
> +	  }
> +    }
> +  else
> +    {
> +      counts = XCNEWVEC (int, nreg);
> +      for (insn = insns; insn; insn = NEXT_INSN (insn))
> +	if (INSN_P (insn))
> +	  count_reg_usage (insn, counts, NULL_RTX, 1);
> +    }

Add a comment about what COUNTS will contain at the end of the computation.


> +	      if (MAY_HAVE_DEBUG_INSNS
> +		  && (set = single_set (insn)) != NULL_RTX
> +		  && is_dead_reg (SET_DEST (set), counts)
> +		  /* Used at least once in some DEBUG_INSN.  */
> +		  && counts[REGNO (SET_DEST (set)) + nreg] > 0
> +		  /* And set exactly once.  */
> +		  && counts[REGNO (SET_DEST (set)) + nreg * 2] == 1
> +		  && !side_effects_p (SET_SRC (set))
> +		  && asm_noperands (PATTERN (insn)) < 0)
> +		{
> +		  rtx dval, bind;
> +
> +		  /* Create DEBUG_EXPR (and DEBUG_EXPR_DECL).  */
> +		  dval = make_debug_expr_from_rtl (SET_DEST (set));
> +
> +		  /* Emit a debug bind insn before the insn in which
> +		     reg dies.  */
> +		  bind = gen_rtx_VAR_LOCATION (GET_MODE (SET_DEST (set)),
> +					       DEBUG_EXPR_TREE_DECL (dval),
> +					       SET_SRC (set),
> +					       VAR_INIT_STATUS_INITIALIZED);
> +		  count_reg_usage (bind, counts + nreg, NULL_RTX, 1);
> +
> +		  bind = emit_debug_insn_before (bind, insn);
> +		  df_insn_rescan (bind);
> +
> +		  if (replacements == NULL)
> +		    replacements = XCNEWVEC (rtx, nreg);
> +		  replacements[REGNO (SET_DEST (set))] = dval;
> +		}
> +
> +	      count_reg_usage (insn, counts, NULL_RTX, -1);
> +	      ndead++;
> +	    }
>  	  delete_insn_and_edges (insn);
> -	  ndead++;
>  	}

Add a comment about the high-level purpose of the code (e.g. the ChangeLog).
H.J. Lu Dec. 2, 2010, 11 p.m. UTC | #2 
On Tue, Oct 5, 2010 at 1:48 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> This patch implements something similar to what we do in DF to avoid
> resetting debug stmts, in particular if possible put a D#N temporary
> right before the to be deleted trivially dead insn and use the D#N
> DEBUG_EXPR in following DEBUG_INSNs instead of resetting them.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux.  Ok for trunk?
>
> In i686-linux bootstrap/regtest delete_trivially_dead_insns had
> to reset 18291 DEBUG_INSNs, but additional 19170 could be kept because of
> this patch, on x86_64-linux 20807 resets happened and 3503 could be kept.
> Still, even on x86_64-linux 180 DIEs in cc1plus got newly DW_AT_location
> which they didn't have before.
>
> 2010-10-05  Jakub Jelinek  <jakub@redhat.com>
>
>        * cse.c (is_dead_reg): Change into inline function that is not
>        called through for_each_rtx.
>        (set_live_p): Adjust caller.
>        (insn_live_p): Don't reset DEBUG_INSNs here.
>        (struct dead_debug_insn_data): New data.
>        (count_stores, is_dead_debug_insn, replace_dead_reg): New functions.
>        (delete_trivially_dead_insns): If there is just one setter for the
>        dead reg that is referenced by some DEBUG_INSNs, create a DEBUG_EXPR
>        and add DEBUG_INSN for it right before the removed setter and
>        use the DEBUG_EXPR instead of the dead pseudo.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46771

H.J.
diff mbox

Patch

--- gcc/cse.c.jj	2010-10-01 12:39:13.000000000 +0200
+++ gcc/cse.c	2010-10-05 19:01:34.000000000 +0200
@@ -6687,12 +6687,9 @@  count_reg_usage (rtx x, int *counts, rtx
 
 /* Return true if a register is dead.  Can be used in for_each_rtx.  */
 
-static int
-is_dead_reg (rtx *loc, void *data)
+static inline int
+is_dead_reg (rtx x, int *counts)
 {
-  rtx x = *loc;
-  int *counts = (int *)data;
-
   return (REG_P (x)
 	  && REGNO (x) >= FIRST_PSEUDO_REGISTER
 	  && counts[REGNO (x)] == 0);
@@ -6718,7 +6715,7 @@  set_live_p (rtx set, rtx insn ATTRIBUTE_
 	       || !reg_referenced_p (cc0_rtx, PATTERN (tem))))
     return false;
 #endif
-  else if (!is_dead_reg (&SET_DEST (set), counts)
+  else if (!is_dead_reg (SET_DEST (set), counts)
 	   || side_effects_p (SET_SRC (set)))
     return true;
   return false;
@@ -6762,21 +6759,67 @@  insn_live_p (rtx insn, int *counts)
 	else if (INSN_VAR_LOCATION_DECL (insn) == INSN_VAR_LOCATION_DECL (next))
 	  return false;
 
-      /* If this debug insn references a dead register, drop the
-	 location expression for now.  ??? We could try to find the
-	 def and see if propagation is possible.  */
-      if (for_each_rtx (&INSN_VAR_LOCATION_LOC (insn), is_dead_reg, counts))
-	{
-	  INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC ();
-	  df_insn_rescan (insn);
-	}
-
       return true;
     }
   else
     return true;
 }
 
+/* Count the number of stores into pseudo.  */
+
+static void
+count_stores (rtx x, const_rtx set ATTRIBUTE_UNUSED, void *data)
+{
+  int *counts = (int *) data;
+  if (REG_P (x) && REGNO (x) >= FIRST_PSEUDO_REGISTER)
+    counts[REGNO (x)]++;
+}
+
+struct dead_debug_insn_data
+{
+  int *counts;
+  rtx *replacements;
+  bool seen_repl;
+};
+
+/* Return if a DEBUG_INSN needs to be reset because some dead
+   pseudo doesn't have a replacement.  */
+
+static int
+is_dead_debug_insn (rtx *loc, void *data)
+{
+  rtx x = *loc;
+  struct dead_debug_insn_data *ddid = (struct dead_debug_insn_data *) data;
+
+  if (is_dead_reg (x, ddid->counts))
+    {
+      if (ddid->replacements && ddid->replacements[REGNO (x)] != NULL_RTX)
+	ddid->seen_repl = true;
+      else
+	return 1;
+    }
+  return 0;
+}
+
+/* Replace a dead pseudo in a DEBUG_INSN with replacement DEBUG_EXPR.  */
+
+static rtx
+replace_dead_reg (rtx x, const_rtx old_rtx ATTRIBUTE_UNUSED, void *data)
+{
+  rtx *replacements = (rtx *) data;
+
+  if (REG_P (x)
+      && REGNO (x) >= FIRST_PSEUDO_REGISTER
+      && replacements[REGNO (x)] != NULL_RTX)
+    {
+      if (GET_MODE (x) == GET_MODE (replacements[REGNO (x)]))
+	return replacements[REGNO (x)];
+      return lowpart_subreg (GET_MODE (x), replacements[REGNO (x)],
+			     GET_MODE (replacements[REGNO (x)]));
+    }
+  return NULL_RTX;
+}
+
 /* Scan all the insns and delete any that are dead; i.e., they store a register
    that is never used or they copy a register to itself.
 
@@ -6790,15 +6833,31 @@  delete_trivially_dead_insns (rtx insns, 
 {
   int *counts;
   rtx insn, prev;
+  rtx *replacements = NULL;
   int ndead = 0;
 
   timevar_push (TV_DELETE_TRIVIALLY_DEAD);
   /* First count the number of times each register is used.  */
-  counts = XCNEWVEC (int, nreg);
-  for (insn = insns; insn; insn = NEXT_INSN (insn))
-    if (INSN_P (insn))
-      count_reg_usage (insn, counts, NULL_RTX, 1);
-
+  if (MAY_HAVE_DEBUG_INSNS)
+    {
+      counts = XCNEWVEC (int, nreg * 3);
+      for (insn = insns; insn; insn = NEXT_INSN (insn))
+	if (DEBUG_INSN_P (insn))
+	  count_reg_usage (INSN_VAR_LOCATION_LOC (insn), counts + nreg,
+			   NULL_RTX, 1);
+	else if (INSN_P (insn))
+	  {
+	    count_reg_usage (insn, counts, NULL_RTX, 1);
+	    note_stores (PATTERN (insn), count_stores, counts + nreg * 2);
+	  }
+    }
+  else
+    {
+      counts = XCNEWVEC (int, nreg);
+      for (insn = insns; insn; insn = NEXT_INSN (insn))
+	if (INSN_P (insn))
+	  count_reg_usage (insn, counts, NULL_RTX, 1);
+    }
   /* Go from the last insn to the first and delete insns that only set unused
      registers or copy a register to itself.  As we delete an insn, remove
      usage counts for registers it uses.
@@ -6821,12 +6880,80 @@  delete_trivially_dead_insns (rtx insns, 
 
       if (! live_insn && dbg_cnt (delete_trivial_dead))
 	{
-	  count_reg_usage (insn, counts, NULL_RTX, -1);
+	  if (DEBUG_INSN_P (insn))
+	    count_reg_usage (INSN_VAR_LOCATION_LOC (insn), counts + nreg,
+			     NULL_RTX, -1);
+	  else
+	    {
+	      rtx set;
+	      if (MAY_HAVE_DEBUG_INSNS
+		  && (set = single_set (insn)) != NULL_RTX
+		  && is_dead_reg (SET_DEST (set), counts)
+		  /* Used at least once in some DEBUG_INSN.  */
+		  && counts[REGNO (SET_DEST (set)) + nreg] > 0
+		  /* And set exactly once.  */
+		  && counts[REGNO (SET_DEST (set)) + nreg * 2] == 1
+		  && !side_effects_p (SET_SRC (set))
+		  && asm_noperands (PATTERN (insn)) < 0)
+		{
+		  rtx dval, bind;
+
+		  /* Create DEBUG_EXPR (and DEBUG_EXPR_DECL).  */
+		  dval = make_debug_expr_from_rtl (SET_DEST (set));
+
+		  /* Emit a debug bind insn before the insn in which
+		     reg dies.  */
+		  bind = gen_rtx_VAR_LOCATION (GET_MODE (SET_DEST (set)),
+					       DEBUG_EXPR_TREE_DECL (dval),
+					       SET_SRC (set),
+					       VAR_INIT_STATUS_INITIALIZED);
+		  count_reg_usage (bind, counts + nreg, NULL_RTX, 1);
+
+		  bind = emit_debug_insn_before (bind, insn);
+		  df_insn_rescan (bind);
+
+		  if (replacements == NULL)
+		    replacements = XCNEWVEC (rtx, nreg);
+		  replacements[REGNO (SET_DEST (set))] = dval;
+		}
+
+	      count_reg_usage (insn, counts, NULL_RTX, -1);
+	      ndead++;
+	    }
 	  delete_insn_and_edges (insn);
-	  ndead++;
 	}
     }
 
+  if (MAY_HAVE_DEBUG_INSNS)
+    {
+      struct dead_debug_insn_data ddid;
+      ddid.counts = counts;
+      ddid.replacements = replacements;
+      for (insn = get_last_insn (); insn; insn = PREV_INSN (insn))
+	if (DEBUG_INSN_P (insn))
+	  {
+	    /* If this debug insn references a dead register that wasn't replaced
+	       with an DEBUG_EXPR, reset the DEBUG_INSN.  */
+	    ddid.seen_repl = false;
+	    if (for_each_rtx (&INSN_VAR_LOCATION_LOC (insn),
+			      is_dead_debug_insn, &ddid))
+	      {
+		INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC ();
+		df_insn_rescan (insn);
+	      }
+	    else if (ddid.seen_repl)
+	      {
+		INSN_VAR_LOCATION_LOC (insn)
+		  = simplify_replace_fn_rtx (INSN_VAR_LOCATION_LOC (insn),
+					     NULL_RTX, replace_dead_reg,
+					     replacements);
+		df_insn_rescan (insn);
+	      }
+	  }
+      if (replacements)
+	free (replacements);
+    }
+
   if (dump_file && ndead)
     fprintf (dump_file, "Deleted %i trivially dead insns\n",
 	     ndead);