Patchwork fix PR52139 correctly

login
register
mail settings
Submitter Steven Bosscher
Date April 13, 2013, 6:21 p.m.
Message ID <CABu31nOmQp=VN_YvhWFSQT-rJGd5vgAU1ZUH_ZuBocNdq1wUMg@mail.gmail.com>
Download mbox | patch
Permalink /patch/236387/
State New
Headers show

Comments

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

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;
     }