fix PR52139 correctly

Submitted by Steven Bosscher on April 13, 2013, 6:21 p.m.

Details

Message ID CABu31nOmQp=VN_YvhWFSQT-rJGd5vgAU1ZUH_ZuBocNdq1wUMg@mail.gmail.com
State New
Headers show

Commit Message

Steven Bosscher April 13, 2013, 6:21 p.m.
Hello,

The fix for PR52139 only papered over another problem: That things
from a basic block header were emitted into the insns stream. When
going out of cfglayout mode, these header-insn will be lost. It's
probably possible to construct a test case where e.g. a
NOTE_INSN_DELETED_DEBUG_LABEL is lost because of this, but I haven't
tried to do so.

The correct fix is to find a new home for the header and footer insn,
and the most logical place is to put them in the footer of the merged
block.

Bootstrapped&tested on x86_64-unknown-linux-gnu with current trunk,
and with r184004 (modified patch) to make sure the test case is fixed
(it doesn't fail on trunk even with r184005 reverted).
OK for trunk?

Ciao!
Steven


        * cfgrtl.c (cfg_layout_merge_blocks): Revert r184005, implement
        correct fix by moving header and footer insn to the footer of
        the merged basic block.  Clear BB_END of the merged-away block.

Comments

Jakub Jelinek May 13, 2013, 7:52 a.m.
On Sat, Apr 13, 2013 at 08:21:46PM +0200, Steven Bosscher wrote:
>         * cfgrtl.c (cfg_layout_merge_blocks): Revert r184005, implement
>         correct fix by moving header and footer insn to the footer of
>         the merged basic block.  Clear BB_END of the merged-away block.

Unfortunately this caused PR57257.

	Jakub

Patch hide | download patch | download mbox

Index: cfgrtl.c
===================================================================
--- cfgrtl.c    (revision 197942)
+++ cfgrtl.c    (working copy)
@@ -4083,18 +4083,40 @@  cfg_layout_merge_blocks (basic_block a,
   if (!optimize)
     emit_nop_for_unique_locus_between (a, b);

-  /* Possible line number notes should appear in between.  */
-  if (BB_HEADER (b))
-    {
-      rtx first = BB_END (a), last;
-
-      last = emit_insn_after_noloc (BB_HEADER (b), BB_END (a), a);
-      /* The above might add a BARRIER as BB_END, but as barriers
-        aren't valid parts of a bb, remove_insn doesn't update
-        BB_END if it is a barrier.  So adjust BB_END here.  */
-      while (BB_END (a) != first && BARRIER_P (BB_END (a)))
-       BB_END (a) = PREV_INSN (BB_END (a));
-      delete_insn_chain (NEXT_INSN (first), last, false);
+  /* Move things from b->footer after a->footer.  */
+  if (BB_FOOTER (b))
+    {
+      if (!BB_FOOTER (a))
+       BB_FOOTER (a) = BB_FOOTER (b);
+      else
+       {
+         rtx last = BB_FOOTER (a);
+
+         while (NEXT_INSN (last))
+           last = NEXT_INSN (last);
+         NEXT_INSN (last) = BB_FOOTER (b);
+         PREV_INSN (BB_FOOTER (b)) = last;
+       }
+      BB_FOOTER (b) = NULL;
+    }
+
+  /* Move things from b->header before a->footer.
+     Note that this may include dead tablejump data, but we don't clean
+     those up until we go out of cfglayout mode.  */
+   if (BB_HEADER (b))
+     {
+      if (! BB_FOOTER (a))
+       BB_FOOTER (a) = BB_HEADER (b);
+      else
+       {
+         rtx last = BB_HEADER (b);
+
+         while (NEXT_INSN (last))
+           last = NEXT_INSN (last);
+         NEXT_INSN (last) = BB_FOOTER (a);
+         PREV_INSN (BB_FOOTER (a)) = last;
+         BB_FOOTER (a) = BB_HEADER (b);
+       }
       BB_HEADER (b) = NULL;
     }