public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] fix PR52139 correctly
@ 2013-04-14 10:58 Steven Bosscher
  2013-05-13  7:52 ` Jakub Jelinek
  0 siblings, 1 reply; 2+ messages in thread
From: Steven Bosscher @ 2013-04-14 10:58 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jakub Jelinek, Richard Biener

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.

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [patch] fix PR52139 correctly
  2013-04-14 10:58 [patch] fix PR52139 correctly Steven Bosscher
@ 2013-05-13  7:52 ` Jakub Jelinek
  0 siblings, 0 replies; 2+ messages in thread
From: Jakub Jelinek @ 2013-05-13  7:52 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: GCC Patches, Richard Biener

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-05-13  7:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-14 10:58 [patch] fix PR52139 correctly Steven Bosscher
2013-05-13  7:52 ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).