[Bug target/59233] [4.9 Regression] C++ failures after revision 205058 on -apple-darwin with -m32

Thu Nov 21 17:32:00 GMT 2013

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59233

--- Comment #6 from Teresa Johnson <tejohnson at google dot com> ---
On Thu, Nov 21, 2013 at 7:10 AM, tejohnson at google dot com
<gcc-bugzilla@gcc.gnu.org> wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59233
>
> --- Comment #5 from Teresa Johnson <tejohnson at google dot com> ---
> Reproduced with cross compiler:
>
> $
> /usr/local/google/home/tejohnson/gcc_trunk_9_x86_64-apple-darwin10/bld-gcc/./gcc/xg++
> -B/usr/local/google/home/tejohnson/gcc_trunk_9_x86_64-apple-darwin10/bld-gcc/./gcc/
> -B/usr/local/google/home/tejohnson/gcc_trunk_9_x86_64-apple-darwin10/install/x86_64-apple-darwin10/bin/
> -B/usr/local/google/home/tejohnson/gcc_trunk_9_x86_64-apple-darwin10/install/x86_64-apple-darwin10/lib/
> -isystem
> /usr/local/google/home/tejohnson/gcc_trunk_9_x86_64-apple-darwin10/install/x86_64-apple-darwin10/include
> -isystem
> /usr/local/google/home/tejohnson/gcc_trunk_9_x86_64-apple-darwin10/install/x86_64-apple-darwin10/sys-include
>    -c -m32 -Os pr52772.C
> pr52772.C: In member function ‘int c8::tria(c7*, c5*)’:
> pr52772.C:85:1: internal compiler error: Segmentation fault
>  }
>  ^
> 0xb657af crash_signal
> /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/toplev.c:336
> 0xff4e1d old_insns_match_p
> /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:1128
> 0xff4e1d old_insns_match_p
> /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:1093
> 0xff5943 outgoing_edges_match
> /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:1754
> 0xff5943 try_crossjump_to_edge
> /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:1927
> 0xff6ed4 try_crossjump_bb
> /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:2252
> 0xff6ed4 try_crossjump_bb
> /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:2138
> 0xff7a6c try_optimize_cfg
> /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:2808
> 0xff7a6c cleanup_cfg(int)
> /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:3014
> 0xff9836 execute_jump2
> /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:3123
> 0xff9836 execute
> /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:3152
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <http://gcc.gnu.org/bugs.html> for instructions.
>
> In debugger:
>
> Program received signal SIGSEGV, Segmentation fault.
> old_insns_match_p (i2=0x7ffff6390cc0, i1=0x7ffff6390d00, mode=2)
>     at /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:1128
> 1128  if (GET_CODE (p1) != GET_CODE (p2))
> (gdb) p p1
> $1 = (rtx) 0x0
> (gdb) p p2
> $2 = (rtx) 0x0
> (gdb) p i1
> $3 = (rtx) 0x7ffff6390d00
> (gdb) p i2
> $4 = (rtx) 0x7ffff6390cc0
> (gdb) p debug_rtx(i1)
> (note 170 134 58 7 NOTE_INSN_DELETED)
> $5 = void
> (gdb) p debug_rtx(i2)
> (note 169 130 87 11 NOTE_INSN_DELETED)
> $6 = void
>
> So it doesn't handle NOTE_INSN_DELETED. Looking at the caller:
>
> (gdb) up
> #1  old_insns_match_p (mode=2, i1=0x7ffff6390d00, i2=0x7ffff6390cc0)
>     at /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:1093
> 1093 old_insns_match_p (int mode ATTRIBUTE_UNUSED, rtx i1, rtx i2)
> (gdb) up
> #2  0x0000000000ff5944 in outgoing_edges_match (bb2=0x7ffff639b270,
>     bb1=0x7ffff639b2d8, mode=2)
>     at /usr/local/google/home/tejohnson/gcc_trunk_9/gcc/cfgcleanup.c:1754
> 1754  if (old_insns_match_p (mode, last1, last2) != dir_both)
> (gdb) p debug_bb(bb1)
> (code_label/s 131 154 134 7 17 "" [1 uses])
> (note 134 131 170 7 [bb 7] NOTE_INSN_BASIC_BLOCK)
> (note 170 134 58 7 NOTE_INSN_DELETED)
>
> $11 = void
> (gdb) p debug_bb(bb2)
> (code_label/s 127 167 130 11 16 "" [1 uses])
> (note 130 127 169 11 [bb 11] NOTE_INSN_BASIC_BLOCK)
> (note 169 130 87 11 NOTE_INSN_DELETED)
>
> $12 = void
>
>
> Looking to see what the right solution is here - either
> old_insns_match_p should handle this or caller outgoing_edges_match
> should avoid calling old_insns_match_p on these instruction types.

I have added handling to outgoing_edges_match to ignore these and
other types of notes that it doesn't understand, just as it was
ignoring debug instructions. This fixes the immediate problem.

But the next question is how does -freorder-blocks-and-partition cause
an issue, since it won't do anything with -Os? The reason is that in
except.c when expanding landing pads, under
flag_reorder_blocks_and_partition the edges are marked with
EDGE_PRESERVE, which means some  block combining upstream of jump2 is
not occurring, resulting in the attempt at optimizing this block. Note
that later on the EDGE_PRESERVE flag is removed by
find_rarely_executed_basic_blocks_and_crossing_edges. However, in this
case since we have -Os, bbpart is gated off, and we never execute
find_rarely_executed_basic_blocks_and_crossing_edges. Additionally, we
can detect earlier during except.c that we will not even attempt
bbpart. In order to do this I have moved the body of
gate_handle_partition_blocks to a new function do_partition_blocks
that can be called by except.c to check whether we really need to
preserve the landing pad edges.

I am currently regression testing both fixes on
x86-64-unknown-linux-gnu. Here is the full patch.

Teresa

2013-11-21  Teresa Johnson  <tejohnson@google.com>

        * bb-reorder.c (do_partition_blocks): New function.
        (gate_handle_partition_blocks): Call do_partition_blocks.
        * bb-reorder.h (do_partition_blocks): Declare.
        * cfgcleanup.c (outgoing_edges_match): Walk up past note instructions
        not understood by old_insns_match_p.
        * except.c (dw2_build_landing_pads): Call do_partition_blocks.

Index: bb-reorder.c
===================================================================
--- bb-reorder.c        (revision 205063)
+++ bb-reorder.c        (working copy)
@@ -2532,8 +2532,13 @@ make_pass_duplicate_computed_gotos (gcc::context *
   return new pass_duplicate_computed_gotos (ctxt);
 }

-static bool
-gate_handle_partition_blocks (void)
+/* This function will return true if hot/cold partitioning will
+   be performed, and is declared non-static so that other passes
+   (namely landing pad expansion in except.c) can more accurately
+   determine whether they need to assume that partitioning will occur.  */
+
+bool
+do_partition_blocks (void)
 {
   /* The optimization to partition hot/cold basic blocks into separate
      sections of the .o file does not work well with linkonce or with
@@ -2548,6 +2553,12 @@ make_pass_duplicate_computed_gotos (gcc::context *
          && !user_defined_section_attribute);
 }

+static bool
+gate_handle_partition_blocks (void)
+{
+  return do_partition_blocks ();
+}
+
 /* This function is the main 'entrance' for the optimization that
    partitions hot and cold basic blocks into separate sections of the
    .o file (to improve performance and cache locality).  Ideally it
Index: bb-reorder.h
===================================================================
--- bb-reorder.h        (revision 205063)
+++ bb-reorder.h        (working copy)
@@ -37,4 +37,5 @@ extern int get_uncond_jump_length (void);

 extern void insert_section_boundary_note (void);

+extern bool do_partition_blocks (void);
 #endif
Index: cfgcleanup.c
===================================================================
--- cfgcleanup.c        (revision 205063)
+++ cfgcleanup.c        (working copy)
@@ -1743,12 +1743,20 @@ outgoing_edges_match (int mode, basic_block bb1, b
        }
     }

+  /* Find the last non-debug non-note instruction in each bb, except
+     stop when we see the NOTE_INSN_BASIC_BLOCK, as old_insns_match_p
+     handles that case specially. old_insns_match_p does not handle
+     other types of instruction notes.  */
   rtx last1 = BB_END (bb1);
   rtx last2 = BB_END (bb2);
-  if (DEBUG_INSN_P (last1))
-    last1 = prev_nondebug_insn (last1);
-  if (DEBUG_INSN_P (last2))
-    last2 = prev_nondebug_insn (last2);
+  while (!NOTE_INSN_BASIC_BLOCK_P (last1) &&
+         (DEBUG_INSN_P (last1) || NOTE_P (last1)))
+    last1 = PREV_INSN (last1);
+  while (!NOTE_INSN_BASIC_BLOCK_P (last2) &&
+         (DEBUG_INSN_P (last2) || NOTE_P (last2)))
+    last2 = PREV_INSN (last2);
+  gcc_assert (last1 && last2);
+
   /* First ensure that the instructions match.  There may be many outgoing
      edges so this test is generally cheaper.  */
   if (old_insns_match_p (mode, last1, last2) != dir_both)
Index: except.c
===================================================================
--- except.c    (revision 205063)
+++ except.c    (working copy)
@@ -125,6 +125,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "except.h"
 #include "hard-reg-set.h"
 #include "basic-block.h"
+#include "bb-reorder.h"
 #include "output.h"
 #include "dwarf2asm.h"
 #include "dwarf2out.h"
@@ -1014,7 +1015,7 @@ dw2_build_landing_pads (void)
      new landing pads later, which means that we need to hold on to
      the post-landing-pad block.  Prevent it from being merged away.
      We'll remove this bit after partitioning.  */
-  if (flag_reorder_blocks_and_partition)
+  if (do_partition_blocks ())
     e_flags |= EDGE_PRESERVE;

   for (i = 1; vec_safe_iterate (cfun->eh->lp_array, i, &lp); ++i)


>
> Teresa
>
> On Thu, Nov 21, 2013 at 6:41 AM, tejohnson at google dot com
> <gcc-bugzilla@gcc.gnu.org> wrote:
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59233
>>
>> --- Comment #3 from Teresa Johnson <tejohnson at google dot com> ---
>> On Thu, Nov 21, 2013 at 6:10 AM, dominiq at lps dot ens.fr
>> <gcc-bugzilla@gcc.gnu.org> wrote:
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59233
>>>
>>> --- Comment #2 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
>>> The ICE with -freorder-blocks-and-partition appeared between revisions 200946
>>> (OK, 2013-07-14) and 201266 (ICE, 2013-07-26).
>>>
>>
>> Taking a look. Since I can't reproduce on x86-64-unknown-linux-gnu I
>> am going to build a cross-compiler for x86_64-apple-darwin10 and see
>> if I can reproduce it. I looked at the range of revisions you mention
>> above and there weren't any partitioning specific changes, so it is
>> probably a side effect of some other change in that range.
>>
>> Teresa
>>
>>> --
>>> You are receiving this mail because:
>>> You are on the CC list for the bug.
>>
>> --
>> You are receiving this mail because:
>> You are on the CC list for the bug.
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.


[Bug target/59233] [4.9 Regression] C++ failures after revision 205058 on *-apple-darwin* with -m32

[Bug target/59233] [4.9 Regression] C++ failures after revision 205058 on -apple-darwin with -m32