Message ID | 4EDAB22E.2080204@mentor.com |
---|---|
State | New |
Headers | show |
Tom de Vries <Tom_deVries@mentor.com> writes: > OK, factored out delete_label now. > > Bootstrapped and reg-tested on x86_64. > > Ok for next stage1? Looks good codewise. I'm just a bit worried about the name "delete_label". "delete_insn (label)" should always do the right thing for a pure deletion; the point of the new routine is that it also moves instructions. I'd prefer a name that differentiated it from delete_insn. E.g. "hide_label" or "decommission_label", although as you can tell I'm useless at naming things... Thanks, Richard
> Looks good codewise. Seconded, modulo the file: the function should be in cfgrtl.c instead. > I'm just a bit worried about the name "delete_label". > "delete_insn (label)" should always do the right thing for a pure deletion; > the point of the new routine is that it also moves instructions. It only fixes things up though, so that the RTL stream is valid again. Hence the question: why not retrofit it into delete_insn directly?
Hi, On Sun, 4 Dec 2011, Eric Botcazou wrote: > > I'm just a bit worried about the name "delete_label". "delete_insn > > (label)" should always do the right thing for a pure deletion; the > > point of the new routine is that it also moves instructions. > > It only fixes things up though, so that the RTL stream is valid again. > Hence the question: why not retrofit it into delete_insn directly? Was my first reaction too. What good is delete_insn(label) if it leaves the stream in a state to be fixed up immediately. Ciao, Michael.
On 05/12/11 13:50, Michael Matz wrote: > Hi, > > On Sun, 4 Dec 2011, Eric Botcazou wrote: > >>> I'm just a bit worried about the name "delete_label". "delete_insn >>> (label)" should always do the right thing for a pure deletion; the >>> point of the new routine is that it also moves instructions. >> >> It only fixes things up though, so that the RTL stream is valid again. >> Hence the question: why not retrofit it into delete_insn directly? > > Was my first reaction too. What good is delete_insn(label) if it leaves > the stream in a state to be fixed up immediately. > delete_insn returns the next insn. If I have: (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) and delete_insn turns the label into a note and changes order into: (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) what should be the 'next' returned by delete_insn? Thanks, - Tom > > Ciao, > Michael.
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.
Ciao,
Michael.
Index: gcc/cfgcleanup.c =================================================================== --- gcc/cfgcleanup.c (revision 181172) +++ gcc/cfgcleanup.c (working copy) @@ -2518,6 +2518,39 @@ trivially_empty_bb_p (basic_block bb) } } +/* Delete the label at the head of BB, and if that results in a DELETED_LABEL + note, move it after the BASIC_BLOCK note of BB. */ + +void +delete_label (basic_block bb) +{ + rtx label = BB_HEAD (bb), deleted_label, bb_note; + gcc_assert (LABEL_P (label)); + + /* Delete the label. */ + delete_insn (label); + if (dump_file) + fprintf (dump_file, "Deleted label in block %i.\n", + bb->index); + + /* Find the DELETED_LABEL note. */ + deleted_label = BB_HEAD (bb); + if (deleted_label == NULL_RTX + || !NOTE_P (deleted_label) + || NOTE_KIND (deleted_label) != NOTE_INSN_DELETED_LABEL) + return; + + /* Find the BASIC_BLOCK note. */ + bb_note = NEXT_INSN (deleted_label); + gcc_assert (bb_note != NULL_RTX && NOTE_INSN_BASIC_BLOCK_P (bb_note)); + + /* Move the DELETED_LABEL note after the BASIC_BLOCK 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. */ @@ -2631,25 +2664,7 @@ try_optimize_cfg (int mode) || !JUMP_P (BB_END (single_pred (b))) || ! 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; - } - if (dump_file) - fprintf (dump_file, "Deleted label in block %i.\n", - b->index); - } + delete_label (b); /* If we fall through an empty block, we can remove it. */ if (!(mode & CLEANUP_CFGLAYOUT) Index: gcc/cfglayout.c =================================================================== --- gcc/cfglayout.c (revision 181172) +++ 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_label (e_taken->dest); continue; } } Index: gcc/basic-block.h =================================================================== --- gcc/basic-block.h (revision 181172) +++ 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 delete_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" } } */ +