diff mbox

Remove dead labels to increase superblock scope

Message ID 4ECF70A6.4040200@mentor.com
State New
Headers show

Commit Message

Tom de Vries Nov. 25, 2011, 10:40 a.m. UTC 
On 21/11/11 17:13, Michael Matz wrote:
> 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.

Ok, using that in the new version.

> 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.
>

I tried that as well but ran into these errors in rtl_verify_flow_info_1:
...
libquadmath/printf/cmp.c:56:1: error: NOTE_INSN_BASIC_BLOCK is missing for block 6
libquadmath/printf/cmp.c:56:1: error: NOTE_INSN_BASIC_BLOCK 79 in middle of
basic block 6
libquadmath/printf/cmp.c:56:1: internal compiler error: verify_flow_info failed
a-direct.ads:460:9: error: NOTE_INSN_BASIC_BLOCK is missing for block 6
a-direct.ads:460:9: error: NOTE_INSN_BASIC_BLOCK 25 in middle of basic block 6
+===========================GNAT BUG DETECTED==============================+
| 4.7.0 20111123 (experimental) (x86_64-unknown-linux-gnu) GCC error:      |
| verify_flow_info failed                                                  |
| Error detected around a-direct.ads:460:9                                 |
...

Eric,

This new patch was bootstrapped and reg-tested on x86_64.

this new patch or old patch (
http://gcc.gnu.org/ml/gcc-patches/2011-11/msg01953.html ) ok for next stage1?

Thanks,
- Tom

> 
> Ciao,
> Michael.

2011-11-25  Tom de Vries  <tom@codesourcery.com>

	* rtl.h (can_delete_label_p): Declare.
	* cfgrtl.c (can_delete_label_p): Remove static.
	* cfglayout.c (fixup_reorder_chain): Delete unused label if
	can_delete_label_p.

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

Comments

Michael Matz Nov. 25, 2011, 1:03 p.m. UTC | #1 
Hi,

On Fri, 25 Nov 2011, Tom de Vries wrote:

> > 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.
> 
> I tried that as well but ran into these errors in rtl_verify_flow_info_1:
> ...
> libquadmath/printf/cmp.c:56:1: error: NOTE_INSN_BASIC_BLOCK is missing for block 6
> libquadmath/printf/cmp.c:56:1: error: NOTE_INSN_BASIC_BLOCK 79 in middle of
> basic block 6

Hmpf, probably bitrotted over time.  Oh well, so be it.


Ciao,
Michael.
Steven Bosscher Nov. 25, 2011, 1:05 p.m. UTC | #2 
On Fri, Nov 25, 2011 at 2:03 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Fri, 25 Nov 2011, Tom de Vries wrote:
>
>> > 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.
>>
>> I tried that as well but ran into these errors in rtl_verify_flow_info_1:
>> ...
>> libquadmath/printf/cmp.c:56:1: error: NOTE_INSN_BASIC_BLOCK is missing for block 6
>> libquadmath/printf/cmp.c:56:1: error: NOTE_INSN_BASIC_BLOCK 79 in middle of
>> basic block 6
>
> Hmpf, probably bitrotted over time.  Oh well, so be it.

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...

Ciao!
Steven
Eric Botcazou Nov. 27, 2011, 10:59 p.m. UTC | #3 
> 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.
diff mbox

Patch

Index: gcc/cfglayout.c
===================================================================
--- gcc/cfglayout.c (revision 181652)
+++ gcc/cfglayout.c (working copy)
@@ -857,6 +857,10 @@  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
+		      && can_delete_label_p (ret_label)
+		      && 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)
@@ -2482,6 +2482,9 @@  extern void dump_combine_total_stats (FI
 /* In cfgcleanup.c  */
 extern void delete_dead_jumptables (void);
 
+/* In rtlcfg.c  */
+int can_delete_label_p (const_rtx);
+
 /* In sched-vis.c.  */
 extern void debug_bb_n_slim (int);
 extern void debug_bb_slim (struct basic_block_def *);
Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c (revision 181652)
+++ gcc/cfgrtl.c (working copy)
@@ -66,7 +66,6 @@  along with GCC; see the file COPYING3.
 #include "df.h"
 
 static int can_delete_note_p (const_rtx);
-static int can_delete_label_p (const_rtx);
 static basic_block rtl_split_edge (edge);
 static bool rtl_move_block_after (basic_block, basic_block);
 static int rtl_verify_flow_info (void);
@@ -102,7 +101,7 @@  can_delete_note_p (const_rtx note)
 
 /* True if a given label can be deleted.  */
 
-static int
+int
 can_delete_label_p (const_rtx label)
 {
   return (!LABEL_PRESERVE_P (label)
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" } } */
+