Patchwork Remove dead labels to increase superblock scope

login
register
mail settings
Submitter Tom de Vries
Date Dec. 1, 2011, 8:09 a.m.
Message ID <4ED7364D.2070108@mentor.com>
Download mbox | patch
Permalink /patch/128660/
State New
Headers show

Comments

Tom de Vries - Dec. 1, 2011, 8:09 a.m.
On 27/11/11 23:59, Eric Botcazou wrote:
>> No, DELETED_LABEL notes still work just fine. It depends on how you
>> remove the label and replace it with a note, and Tom isn't showing
>> what he did, so...
> 
> I agree that there is no obvious reason why just calling delete_insn would not 
> work, so this should be investigated first.
> 

The reason it didn't work, is because after turning a label into a
NOTE_INSN_DELETED_LABEL, one needs to move it to after the NOTE_INSN_BASIC_BLOCK
as in cfgcleanup.c:try_optimize_cfg():
...
		  delete_insn_chain (label, label, false);
		  /* If the case label is undeletable, move it after the
		     BASIC_BLOCK note.  */
		  if (NOTE_KIND (BB_HEAD (b)) == NOTE_INSN_DELETED_LABEL)
		    {
		      rtx bb_note = NEXT_INSN (BB_HEAD (b));

		      reorder_insns_nobb (label, label, bb_note);
		      BB_HEAD (b) = bb_note;
		      if (BB_END (b) == bb_note)
			BB_END (b) = label;
		    }
...

Attached patch factors out this piece of code and reuses it in fixup_reorder_chain.

Bootstrapped and reg-tested on x86_64.

OK for next stage1?

Thanks,
- Tom

2011-12-01  Tom de Vries  <tom@codesourcery.com>

	* cfgcleanup.c (fixup_deleted_label): New function, factored out of ...
	(try_optimize_cfg): Use fixup_deleted_label.
	* cfglayout.c (fixup_reorder_chain): Delete unused label, and fixup
	using fixup_deleted_label.

	* gcc.dg/superblock.c: New test.
Richard Sandiford - Dec. 2, 2011, 10:40 a.m.
Tom de Vries <Tom_deVries@mentor.com> writes:
> On 27/11/11 23:59, Eric Botcazou wrote:
>>> No, DELETED_LABEL notes still work just fine. It depends on how you
>>> remove the label and replace it with a note, and Tom isn't showing
>>> what he did, so...
>> 
>> I agree that there is no obvious reason why just calling delete_insn would not 
>> work, so this should be investigated first.
>> 
>
> The reason it didn't work, is because after turning a label into a
> NOTE_INSN_DELETED_LABEL, one needs to move it to after the NOTE_INSN_BASIC_BLOCK
> as in cfgcleanup.c:try_optimize_cfg():
> ...
> 		  delete_insn_chain (label, label, false);
> 		  /* If the case label is undeletable, move it after the
> 		     BASIC_BLOCK note.  */
> 		  if (NOTE_KIND (BB_HEAD (b)) == NOTE_INSN_DELETED_LABEL)
> 		    {
> 		      rtx bb_note = NEXT_INSN (BB_HEAD (b));
>
> 		      reorder_insns_nobb (label, label, bb_note);
> 		      BB_HEAD (b) = bb_note;
> 		      if (BB_END (b) == bb_note)
> 			BB_END (b) = label;
> 		    }
> ...
>
> Attached patch factors out this piece of code and reuses it in fixup_reorder_chain.

But isn't...

> @@ -2637,15 +2658,7 @@ try_optimize_cfg (int mode)
>  		  delete_insn_chain (label, label, false);
>  		  /* If the case label is undeletable, move it after the
>  		     BASIC_BLOCK note.  */
> -		  if (NOTE_KIND (BB_HEAD (b)) == NOTE_INSN_DELETED_LABEL)
> -		    {
> -		      rtx bb_note = NEXT_INSN (BB_HEAD (b));
> -
> -		      reorder_insns_nobb (label, label, bb_note);
> -		      BB_HEAD (b) = bb_note;
> -		      if (BB_END (b) == bb_note)
> -			BB_END (b) = label;
> -		    }
> +		  fixup_deleted_label (b);

...this "delete_insn_chain (label, label, false);" call equivalent
to "delete_insn (label)"?  Splitting the operation in two here and:

> Index: gcc/cfglayout.c
> ===================================================================
> --- gcc/cfglayout.c (revision 181652)
> +++ gcc/cfglayout.c (working copy)
> @@ -857,6 +857,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
> +		      && single_pred_p (e_taken->dest))
> +		    {
> +		      delete_insn (ret_label);
> +		      fixup_deleted_label (e_taken->dest);
> +		    }

...here seems a little odd.

Richard

Patch

Index: gcc/cfgcleanup.c
===================================================================
--- gcc/cfgcleanup.c (revision 181652)
+++ gcc/cfgcleanup.c (working copy)
@@ -2518,6 +2518,27 @@  trivially_empty_bb_p (basic_block bb)
     }
 }
 
+/* Move a DELETED_LABEL note after the BASIC_BLOCK note of BB.  */
+
+void
+fixup_deleted_label (basic_block bb)
+{
+  rtx deleted_label = BB_HEAD (bb), bb_note;
+
+  if (deleted_label == NULL_RTX
+      || !NOTE_P (deleted_label)
+      || NOTE_KIND (deleted_label) != NOTE_INSN_DELETED_LABEL)
+    return;
+
+  bb_note = NEXT_INSN (deleted_label);
+  gcc_assert (bb_note != NULL_RTX && NOTE_INSN_BASIC_BLOCK_P (bb_note));
+
+  reorder_insns_nobb (deleted_label, deleted_label, bb_note);
+  BB_HEAD (bb) = bb_note;
+  if (BB_END (bb) == bb_note)
+    BB_END (bb) = deleted_label;
+}
+
 /* Do simple CFG optimizations - basic block merging, simplifying of jump
    instructions etc.  Return nonzero if changes were made.  */
 
@@ -2637,15 +2658,7 @@  try_optimize_cfg (int mode)
 		  delete_insn_chain (label, label, false);
 		  /* If the case label is undeletable, move it after the
 		     BASIC_BLOCK note.  */
-		  if (NOTE_KIND (BB_HEAD (b)) == NOTE_INSN_DELETED_LABEL)
-		    {
-		      rtx bb_note = NEXT_INSN (BB_HEAD (b));
-
-		      reorder_insns_nobb (label, label, bb_note);
-		      BB_HEAD (b) = bb_note;
-		      if (BB_END (b) == bb_note)
-			BB_END (b) = label;
-		    }
+		  fixup_deleted_label (b);
 		  if (dump_file)
 		    fprintf (dump_file, "Deleted label in block %i.\n",
 			     b->index);
Index: gcc/cfglayout.c
===================================================================
--- gcc/cfglayout.c (revision 181652)
+++ gcc/cfglayout.c (working copy)
@@ -857,6 +857,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
+		      && single_pred_p (e_taken->dest))
+		    {
+		      delete_insn (ret_label);
+		      fixup_deleted_label (e_taken->dest);
+		    }
 		  continue;
 		}
 	    }
Index: gcc/basic-block.h
===================================================================
--- gcc/basic-block.h (revision 181652)
+++ gcc/basic-block.h (working copy)
@@ -813,6 +813,7 @@  extern void rtl_make_eh_edge (sbitmap, b
 enum replace_direction { dir_none, dir_forward, dir_backward, dir_both };
 
 /* In cfgcleanup.c.  */
+extern void fixup_deleted_label (basic_block);
 extern bool cleanup_cfg (int);
 extern int flow_find_cross_jump (basic_block, basic_block, rtx *, rtx *,
                                  enum replace_direction*);
Index: gcc/testsuite/gcc.dg/superblock.c
===================================================================
--- /dev/null (new file)
+++ gcc/testsuite/gcc.dg/superblock.c (revision 0)
@@ -0,0 +1,23 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-asynchronous-unwind-tables -fsched2-use-superblocks -fdump-rtl-sched2 -fdump-rtl-bbro" } */
+
+typedef int aligned __attribute__ ((aligned (64)));
+extern void abort (void);
+
+int bar (void *p);
+
+void
+foo (void)
+{
+  char *p = __builtin_alloca (13);
+  aligned i;
+
+  if (bar (p) || bar (&i))
+    abort ();
+}
+
+/* { dg-final { scan-rtl-dump-times "0 uses" 0 "bbro"} } */
+/* { dg-final { scan-rtl-dump-times "ADVANCING TO" 2 "sched2"} } */
+/* { dg-final { cleanup-rtl-dump "bbro" } } */
+/* { dg-final { cleanup-rtl-dump "sched2" } } */
+