diff mbox

Remove dead labels to increase superblock scope

Message ID 4EDF69D4.10809@mentor.com
State New
Headers show

Commit Message

Tom de Vries Dec. 7, 2011, 1:27 p.m. UTC
On 06/12/11 14:25, Michael Matz wrote:
> Hi,
> 
> On Tue, 6 Dec 2011, Tom de Vries wrote:
> 
>> what should be the 'next' returned by delete_insn?
> 
> There are exactly two calls of delete_insn that take the return value.  
> One (delete_insn_and_edges) just uses it to return it itself (and there 
> are no calls to delete_insn_and_edges that use the returned value), the 
> other (delete_insn_chain) wants to have the original next insn (before 
> reordering), even if that means that it can see the insn twice (once as 
> label, once as note, the latter would be skipped).
> 
> So, return the original next one.  Even better would be to somehow clean 
> up the single last use of the return value in delete_insn_chain, and make 
> delete_insn return nothing.
> 

I did that now. The only problem I ran into was an assert in remove_insn:
...
      if (BB_HEAD (bb) == insn)
	{
	  /* Never ever delete the basic block note without deleting whole
	     basic block.  */
	  gcc_assert (!NOTE_P (insn));
	  BB_HEAD (bb) = next;
	}
...

The problematic case was a removing a basic_block using delete_insn_chain:
...
  (code_label 33 26 34 4 1 "" [0 uses])

  (note 34 33 50 4 [bb 4] NOTE_INSN_BASIC_BLOCK)

  (note 50 34 51 4 NOTE_INSN_EPILOGUE_BEG)
...

Normally, first the code_label was replaced by a note, then the BASIC_BLOCK note
was removed. The assert would not trigger while removing this note because the
note was not BB_HEAD.

With the fixup, when removing the BASIC_BLOCK note, the note would be at the
head and the assert would trigger:
...
  (note 34 33 50 4 [bb 4] NOTE_INSN_BASIC_BLOCK)

  (note 33 26 34 4 1 "" NOTE_INSN_DELETED_LABEL)

  (note 50 34 51 4 NOTE_INSN_EPILOGUE_BEG)
...

I solved this by removing backwards rather than forwards in delete_insn_chain.

Bootstrapped and reg-tested on x86_64.

OK for next stage1?

Thanks,
- Tom

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

	* cfgcleanup.c (try_optimize_cfg): Replace call to delete_isns_chain by
	call to delete_insn.  Remove code to reorder BASIC_BLOCK note and
	DELETED_LABEL note, and move it to ...
	* cfg_rtl.c (delete_insn):  Change return type to void.
	(delete_insn_and_edges): Likewise.
	(delete_insn_chain): Handle new return type of delete_insn.  Delete
	chain backwards rather than forwards.
	* rtl.h (delete_insn, delete_insn_and_edges): Change return type to
	void.
	* cfglayout.c (fixup_reorder_chain): Delete unused label.

	* gcc.dg/superblock.c: New test.

Comments

Eric Botcazou Dec. 9, 2011, 1:59 p.m. UTC | #1
> 2011-12-07  Tom de Vries  <tom@codesourcery.com>
>
> 	* cfgcleanup.c (try_optimize_cfg): Replace call to delete_isns_chain by

delete_insn_chain

> 	call to delete_insn.  Remove code to reorder BASIC_BLOCK note and
> 	DELETED_LABEL note, and move it to ...
> 	* cfg_rtl.c (delete_insn):  Change return type to void.

The file is cfgrtl.c without underscore.  When you have a "to..." somewhere, 
you need to have a "...here" somewhere else:

	* cfgrtl.c (delete_insn): ...here.  Change return type to void.

The hunk for delete_insn contains a line with trailing TABs and spaces.

> 	(delete_insn_and_edges): Likewise.
> 	(delete_insn_chain): Handle new return type of delete_insn.  Delete
> 	chain backwards rather than forwards.
> 	* rtl.h (delete_insn, delete_insn_and_edges): Change return type to
> 	void.
> 	* cfglayout.c (fixup_reorder_chain): Delete unused label.
>
> 	* gcc.dg/superblock.c: New test.

OK for stage1 when it re-opens (modulo the above nits).  Nice cleanup!
diff mbox

Patch

Index: gcc/cfgcleanup.c
===================================================================
--- gcc/cfgcleanup.c (revision 181652)
+++ gcc/cfgcleanup.c (working copy)
@@ -2632,20 +2632,7 @@  try_optimize_cfg (int mode)
 		      || ! label_is_jump_target_p (BB_HEAD (b),
 						   BB_END (single_pred (b)))))
 		{
-		  rtx label = BB_HEAD (b);
-
-		  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;
-		    }
+		  delete_insn (BB_HEAD (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,9 @@  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);
 		  continue;
 		}
 	    }
Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h (revision 181652)
+++ gcc/rtl.h (working copy)
@@ -2460,12 +2460,12 @@  extern void add_insn_before (rtx, rtx, s
 extern void add_insn_after (rtx, rtx, struct basic_block_def *);
 extern void remove_insn (rtx);
 extern rtx emit (rtx);
-extern rtx delete_insn (rtx);
+extern void delete_insn (rtx);
 extern rtx entry_of_function (void);
 extern void emit_insn_at_entry (rtx);
 extern void delete_insn_chain (rtx, rtx, bool);
 extern rtx unlink_insn_chain (rtx, rtx);
-extern rtx delete_insn_and_edges (rtx);
+extern void delete_insn_and_edges (rtx);
 extern rtx gen_lowpart_SUBREG (enum machine_mode, rtx);
 extern rtx gen_const_mem (enum machine_mode, rtx);
 extern rtx gen_frame_mem (enum machine_mode, rtx);
Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c (revision 181652)
+++ gcc/cfgrtl.c (working copy)
@@ -111,12 +111,11 @@  can_delete_label_p (const_rtx label)
 	  && !in_expr_list_p (forced_labels, label));
 }
 
-/* Delete INSN by patching it out.  Return the next insn.  */
+/* Delete INSN by patching it out.  */
 
-rtx
+void
 delete_insn (rtx insn)
 {
-  rtx next = NEXT_INSN (insn);
   rtx note;
   bool really_delete = true;
 
@@ -128,11 +127,22 @@  delete_insn (rtx insn)
       if (! can_delete_label_p (insn))
 	{
 	  const char *name = LABEL_NAME (insn);
+	  basic_block bb = BLOCK_FOR_INSN (insn);
+	  rtx bb_note = NEXT_INSN (insn);
 
 	  really_delete = false;
 	  PUT_CODE (insn, NOTE);
 	  NOTE_KIND (insn) = NOTE_INSN_DELETED_LABEL;
 	  NOTE_DELETED_LABEL_NAME (insn) = name;
+	  
+	  if (bb_note != NULL_RTX && NOTE_INSN_BASIC_BLOCK_P (bb_note)
+	      && BLOCK_FOR_INSN (bb_note) == bb)
+	    {
+	      reorder_insns_nobb (insn, insn, bb_note);
+	      BB_HEAD (bb) = bb_note;
+	      if (BB_END (bb) == bb_note)
+		BB_END (bb) = insn;
+	    }
 	}
 
       remove_node_from_expr_list (insn, &nonlocal_goto_handler_labels);
@@ -190,26 +200,22 @@  delete_insn (rtx insn)
 	    LABEL_NUSES (label)--;
 	}
     }
-
-  return next;
 }
 
 /* Like delete_insn but also purge dead edges from BB.  */
 
-rtx
+void
 delete_insn_and_edges (rtx insn)
 {
-  rtx x;
   bool purge = false;
 
   if (INSN_P (insn)
       && BLOCK_FOR_INSN (insn)
       && BB_END (BLOCK_FOR_INSN (insn)) == insn)
     purge = true;
-  x = delete_insn (insn);
+  delete_insn (insn);
   if (purge)
     purge_dead_edges (BLOCK_FOR_INSN (insn));
-  return x;
 }
 
 /* Unlink a chain of insns between START and FINISH, leaving notes
@@ -219,25 +225,26 @@  delete_insn_and_edges (rtx insn)
 void
 delete_insn_chain (rtx start, rtx finish, bool clear_bb)
 {
-  rtx next;
+  rtx prev, current;
 
   /* Unchain the insns one by one.  It would be quicker to delete all of these
      with a single unchaining, rather than one at a time, but we need to keep
      the NOTE's.  */
+  current = finish;
   while (1)
     {
-      next = NEXT_INSN (start);
-      if (NOTE_P (start) && !can_delete_note_p (start))
+      prev = PREV_INSN (current);
+      if (NOTE_P (current) && !can_delete_note_p (current))
 	;
       else
-	next = delete_insn (start);
+	delete_insn (current);
 
-      if (clear_bb && !INSN_DELETED_P (start))
-	set_block_for_insn (start, NULL);
+      if (clear_bb && !INSN_DELETED_P (current))
+	set_block_for_insn (current, NULL);
 
-      if (start == finish)
+      if (current == start)
 	break;
-      start = next;
+      current = prev;
     }
 }
 
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" } } */
+