diff mbox

Remove dead labels to increase superblock scope

Message ID 4EC6E724.4090008@mentor.com
State New
Headers show

Commit Message

Tom de Vries Nov. 18, 2011, 11:15 p.m. UTC 
On 11/18/2011 10:29 PM, Eric Botcazou wrote:
>> For the test-case of PR50764, a dead label is introduced by
>> fixup_reorder_chain in cfg_layout_finalize, called from
>> pass_reorder_blocks.
> 
> I presume that there is no reasonable way of preventing fixup_reorder_chain 
> from introducing it or of teaching fixup_reorder_chain to remove it?
> 

This (untested) patch also removes the dead label for the PR, and I think it is
safe.
...
...

But I see 2 potential issues:
- it only catches this case in fixup_reorder_chain. I don't know if there are
  more cases, but the earlier catch-all-afterwards patch surely will catch
  those, this patch probably not.
- I'm not sure if the use count will drop always drop to 0 in
  fixup_reorder_chain, that might only happen after rebuild_jump_labels.

Thanks,
- Tom

Comments

Michael Matz Nov. 21, 2011, 4:13 p.m. UTC | #1 
Hi,

On Sat, 19 Nov 2011, Tom de Vries wrote:

> On 11/18/2011 10:29 PM, Eric Botcazou wrote:
> >> For the test-case of PR50764, a dead label is introduced by
> >> fixup_reorder_chain in cfg_layout_finalize, called from
> >> pass_reorder_blocks.
> > 
> > I presume that there is no reasonable way of preventing fixup_reorder_chain 
> > from introducing it or of teaching fixup_reorder_chain to remove it?
> > 
> 
> This (untested) patch also removes the dead label for the PR, and I 
> think it is safe. ...

cfgrtl.c has already code to delete labels (delete_insn) when appropriate 
(can_delete_label_p).  Perhaps that can be reused somehow.

> Index: cfglayout.c 
> =================================================================== --- 
> cfglayout.c (revision 181377) +++ cfglayout.c (working copy) @@ -702,6 
> +702,21 @@ relink_block_chain (bool stay_in_cfglayo
>  }
>  
> 
> +static bool
> +forced_label_p (rtx label)
> +{
> +  rtx insn, forced_label;
> +  for (insn = forced_labels; insn; insn = XEXP (insn, 1))
> +    {
> +      forced_label = XEXP (insn, 0);
> +      if (!LABEL_P (forced_label))
> +	continue;
> +      if (forced_label == label)
> +	return true;
> +    }
> +  return false;
> +}

That's in_expr_list_p().

> @@ -857,6 +872,12 @@ fixup_reorder_chain (void)
>  				       (e_taken->src, e_taken->dest));
>  		  e_taken->flags |= EDGE_FALLTHRU;
>  		  update_br_prob_note (bb);
> +		  if (LABEL_NUSES (ret_label) == 0

> +		      && !LABEL_PRESERVE_P (ret_label)
> +		      && LABEL_NAME (ret_label) == NULL
> +		      && !forced_label_p (ret_label)

And this is cfgrtl.c:can_delete_label_p.  Note that you actually 
can remove labels also if they are !can_delete_label_p, if you use 
delete_insn (which you do).  It will replace such undeletable labels by a 
DELETED_LABEL note.


Ciao,
Michael.
diff mbox

Patch

Index: cfglayout.c
===================================================================
--- cfglayout.c (revision 181377)
+++ cfglayout.c (working copy)
@@ -702,6 +702,21 @@  relink_block_chain (bool stay_in_cfglayo
 }
 

+static bool
+forced_label_p (rtx label)
+{
+  rtx insn, forced_label;
+  for (insn = forced_labels; insn; insn = XEXP (insn, 1))
+    {
+      forced_label = XEXP (insn, 0);
+      if (!LABEL_P (forced_label))
+	continue;
+      if (forced_label == label)
+	return true;
+    }
+  return false;
+}
+
 /* Given a reorder chain, rearrange the code to match.  */

 static void
@@ -857,6 +872,12 @@  fixup_reorder_chain (void)
 				       (e_taken->src, e_taken->dest));
 		  e_taken->flags |= EDGE_FALLTHRU;
 		  update_br_prob_note (bb);
+		  if (LABEL_NUSES (ret_label) == 0
+		      && !LABEL_PRESERVE_P (ret_label)
+		      && LABEL_NAME (ret_label) == NULL
+		      && !forced_label_p (ret_label)
+		      && single_pred_p (e_taken->dest))
+		    delete_insn (ret_label);
 		  continue;
 		}
 	    }