[Bug rtl-optimization/57451] New: Incorrect debug ranges emitted for -freorder-blocks-and-partition -g

[Bug rtl-optimization/57451] New: Incorrect debug ranges emitted for -freorder-blocks-and-partition -g

[tejohnson at google dot com]

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

            Bug ID: 57451
           Summary: Incorrect debug ranges emitted for
                    -freorder-blocks-and-partition -g
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tejohnson at google dot com

Created attachment 30214
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30214&action=edit
pr49115.C

While fixing problems with -freorder-blocks-and-partition, I found that the
debug ranges being emitted for split functions with -g are incorrect and
leading to assembler errors. I've attached a simple reproducer
(gcc/testsuite/g++.dg/torture/pr49115.C). To reproduce:

$ g++ pr49115.C -O2 -o pr49115 -fprofile-generate
$ pr49115
$ g++ pr49115.C -O2 -o pr49115 -fprofile-use -g -freorder-blocks-and-partition

/tmp/ccStx78Y.s: Assembler messages:
/tmp/ccStx78Y.s:325: Error: can't resolve `.text.unlikely' {.text.unlikely
section} - `.LBB14' {.text.startup section}

The problem is hidden if the last compile step is changed to drop either -g or
-freorder-blocks-and-partition, or by adding -gdwarf-2 (which doesn't trigger
debug_range support).

[Bug rtl-optimization/57451] Incorrect debug ranges emitted for -freorder-blocks-and-partition -g

[tejohnson at google dot com]

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

Teresa Johnson <tejohnson at google dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ccoutant at google dot com,
                   |                            |tejohnson at google dot com

--- Comment #1 from Teresa Johnson <tejohnson at google dot com> ---
Cary, any ideas on how to fix this issue? Thanks, Teresa

[Bug rtl-optimization/57451] Incorrect debug ranges emitted for -freorder-blocks-and-partition -g

[ccoutant at gcc dot gnu.org]

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

Cary Coutant <ccoutant at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ccoutant at gcc dot gnu.org

--- Comment #2 from Cary Coutant <ccoutant at gcc dot gnu.org> ---
I'll take a look, but at first glance it looks like
have_multiple_function_sections isn't being set in dwarf2out.c, which leads it
to assume that it can use "symbol - symbol" expressions in the range lists.

That flag is set by the switch_text_section hook, which is called from
final_scan_insn when it sees a NOTE_INSN_SWITCH_TEXT_SECTIONS insn. When
-freorder-blocks-and-partitions is turned on, is such a NOTE being emitted?

[Bug rtl-optimization/57451] Incorrect debug ranges emitted for -freorder-blocks-and-partition -g

[tejohnson at google dot com]

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

--- Comment #3 from Teresa Johnson <tejohnson at google dot com> ---
Yes, there is a NOTE_INSN_SWITCH_TEXT_SECTIONS note emitted for functions
that are split. In the attached test case the symbol-symbol expression is
being generated across the split boundary of main(), and I confirmed that
the NOTE is there in between the hot/cold sections.

Thanks,
Teresa


On Wed, Jun 12, 2013 at 10:08 AM, ccoutant at gcc dot gnu.org <
gcc-bugzilla@gcc.gnu.org> wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57451
>
> Cary Coutant <ccoutant at gcc dot gnu.org> changed:
>
>            What    |Removed                     |Added
>
> ----------------------------------------------------------------------------
>                  CC|                            |ccoutant at gcc dot
> gnu.org
>
> --- Comment #2 from Cary Coutant <ccoutant at gcc dot gnu.org> ---
> I'll take a look, but at first glance it looks like
> have_multiple_function_sections isn't being set in dwarf2out.c, which
> leads it
> to assume that it can use "symbol - symbol" expressions in the range lists.
>
> That flag is set by the switch_text_section hook, which is called from
> final_scan_insn when it sees a NOTE_INSN_SWITCH_TEXT_SECTIONS insn. When
> -freorder-blocks-and-partitions is turned on, is such a NOTE being emitted?
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
> You reported the bug.
>

[Bug rtl-optimization/57451] Incorrect debug ranges emitted for -freorder-blocks-and-partition -g

[tejohnson at google dot com]

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

--- Comment #5 from Teresa Johnson <tejohnson at google dot com> ---
On Fri, Jun 14, 2013 at 4:19 PM, ccoutant at gcc dot gnu.org
<gcc-bugzilla@gcc.gnu.org> wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57451
>
> --- Comment #4 from Cary Coutant <ccoutant at gcc dot gnu.org> ---
> The problem is a lexical block in main() that appears to be getting split by
> -freorder-blocks-and-partition, but when debug info is emitted during
> rest_of_handle_final(), this particular lexical block still appears to be a
> single block -- BLOCK_FRAGMENT_CHAIN is NULL, so the DWARF output code decides
> that it can emit a DW_AT_low_pc/high_pc pair instead of a DW_AT_ranges. The
> DW_AT_high_pc is now being output relative to DW_AT_low_pc, so we see an
> assembly expression .LBE14 - .LBB14, which are labels attached to the block
> start and block end, and should be in the same section.
>
> Here's the block:
>
> (gdb) p stmt
> $1 = (tree) 0x7ffff5f4c4b0
> (gdb) pt
> warning: Expression is not an assignment (and might have no effect)
>  <block 0x7ffff5f4c4b0 asm_written used
>     vars <var_decl 0x7ffff608bc78 e
>         type <reference_type 0x7ffff609b930 type <record_type 0x7ffff608e9d8
> MyException>
>             sizes-gimplified unsigned DI
>             size <integer_cst 0x7ffff5f24dc0 constant 64>
>             unit size <integer_cst 0x7ffff5f24de0 constant 8>
>             align 64 symtab 0 alias set -1 canonical type 0x7ffff609b930>
>         readonly tree_1 tree_3 unsigned decl_5 DI file pr49115.C line 21 col 25
> size <integer_cst 0x7ffff5f24dc0 64> unit size <integer_cst 0x7ffff5f24de0 8>
>         align 64 context <function_decl 0x7ffff6096f00 main>>
>     supercontext <block 0x7ffff60c00f0 asm_written used
>         vars <var_decl 0x7ffff608bb48 data type <record_type 0x7ffff608ec78
> Data>
>             used tree_1 tree_3 decl_5 SI file pr49115.C line 18 col 8
>             size <integer_cst 0x7ffff5f42140 constant 32>
>             unit size <integer_cst 0x7ffff5f42160 constant 4>
>             align 128 context <function_decl 0x7ffff6096f00 main>
>             (reg/v:SI 64 [ data ])>
>         supercontext <block 0x7ffff5f4c550 asm_written used supercontext
> <function_decl 0x7ffff6096f00 main>
>             subblocks <block 0x7ffff5f4c500 asm_written used vars <var_decl
> 0x7ffff608bb48 data> supercontext <block 0x7ffff5f4c550> subblocks <block
> 0x7ffff5f4c4b0> chain <block 0x7ffff60c00f0>>>>>
>
> (gdb) p stmt->block
> $2 = {base = {code = BLOCK, side_effects_flag = 0, constant_flag = 0,
> addressable_flag = 0,
>     volatile_flag = 0, readonly_flag = 0, asm_written_flag = 1, nowarning_flag
> = 0, visited = 0,
>     used_flag = 1, nothrow_flag = 0, static_flag = 0, public_flag = 0,
> private_flag = 0,
>     protected_flag = 0, deprecated_flag = 0, default_def_flag = 0, u = {bits =
> {lang_flag_0 = 0,
>         lang_flag_1 = 0, lang_flag_2 = 0, lang_flag_3 = 0, lang_flag_4 = 0,
> lang_flag_5 = 0,
>         lang_flag_6 = 0, saturating_flag = 0, unsigned_flag = 0, packed_flag =
> 0, user_align = 0,
>         nameless_flag = 1, spare0 = 0, spare1 = 0, address_space = 0}, length =
> 2048,
>       version = 2048}}, chain = 0x0, abstract_flag = 0, block_num = 14, locus =
> 0,
>   vars = 0x7ffff608bc78, nonlocalized_vars = 0x0, subblocks = 0x0, supercontext
> = 0x7ffff60c00f0,
>   abstract_origin = 0x0, fragment_origin = 0x0, fragment_chain = 0x0}
>
> Here's the fragment of assembly code between .LBB14 and .LBE14:
>
> .LBB14:
>         # pr49115.C:21
>         .loc 1 21 0
>         call    __cxa_begin_catch
> .LVL7:
>         call    __cxa_end_catch
> .LVL8:
>         .p2align 4,,5
> # SUCC: 3 [100.0%]  count:1
>         jmp     .L15
>         .cfi_endproc
>         .section        .text.unlikely
>         .cfi_startproc
>         .cfi_personality 0x3,__gxx_personality_v0
>         .cfi_lsda 0x3,.LLSDAC4
> # BLOCK 6 freq:5000 seq:4
> # PRED: 4 [50.0%]  (CROSSING,CAN_FALLTHRU)
> .L14:
>         .cfi_def_cfa_offset 16
>         .p2align 4,,8
> .LEHB1:
> # SUCC:
>         call    _Unwind_Resume
> .LEHE1:
> .LVL9:
> .LBE14:
> .LBE15:
>         .cfi_endproc
>
> You can see that the block from .LBB14 to .LBE14 has been split across two
> sections. In order for dwarf2out to generate the proper debug info,
> BLOCK_FRAGMENT_CHAIN(stmt) needs to be non-NULL. I'm not sure why that's not
> happening when the block is split.

It looks like this is all done during the final pass when assembly is
being emitted. In final_start_function the NOTE_INSN_BLOCK_{BEG/END}
notes are inserted based on lexical scopes (in
reemit_insn_block_notes). At the end of reemit_insn_block_notes,
reorder_blocks is called to identify blocks references by more than
one NOTE_INSN_BLOCK_{BEG/END}. Any such blocks are duplicated and the
BLOCK_FRAGMENT_CHAIN is setup.

I'm not familiar with this code, but perhaps when a switch section
note is seen by reemit_insn_block_notes, it should invoke change_scope
to emit the necessary NOTE_INSN_BLOCK_* notes?

Thanks,
Teresa

>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
> You reported the bug.



--
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

[Bug rtl-optimization/57451] Incorrect debug ranges emitted for -freorder-blocks-and-partition -g

[tejohnson at google dot com]

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

--- Comment #6 from Teresa Johnson <tejohnson at google dot com> ---
On Sat, Jun 15, 2013 at 7:02 AM, Teresa Johnson <tejohnson@google.com> wrote:
> On Fri, Jun 14, 2013 at 4:19 PM, ccoutant at gcc dot gnu.org
> <gcc-bugzilla@gcc.gnu.org> wrote:
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57451
>>
>> --- Comment #4 from Cary Coutant <ccoutant at gcc dot gnu.org> ---
>> The problem is a lexical block in main() that appears to be getting split by
>> -freorder-blocks-and-partition, but when debug info is emitted during
>> rest_of_handle_final(), this particular lexical block still appears to be a
>> single block -- BLOCK_FRAGMENT_CHAIN is NULL, so the DWARF output code decides
>> that it can emit a DW_AT_low_pc/high_pc pair instead of a DW_AT_ranges. The
>> DW_AT_high_pc is now being output relative to DW_AT_low_pc, so we see an
>> assembly expression .LBE14 - .LBB14, which are labels attached to the block
>> start and block end, and should be in the same section.
>>
>> Here's the block:
>>
>> (gdb) p stmt
>> $1 = (tree) 0x7ffff5f4c4b0
>> (gdb) pt
>> warning: Expression is not an assignment (and might have no effect)
>>  <block 0x7ffff5f4c4b0 asm_written used
>>     vars <var_decl 0x7ffff608bc78 e
>>         type <reference_type 0x7ffff609b930 type <record_type 0x7ffff608e9d8
>> MyException>
>>             sizes-gimplified unsigned DI
>>             size <integer_cst 0x7ffff5f24dc0 constant 64>
>>             unit size <integer_cst 0x7ffff5f24de0 constant 8>
>>             align 64 symtab 0 alias set -1 canonical type 0x7ffff609b930>
>>         readonly tree_1 tree_3 unsigned decl_5 DI file pr49115.C line 21 col 25
>> size <integer_cst 0x7ffff5f24dc0 64> unit size <integer_cst 0x7ffff5f24de0 8>
>>         align 64 context <function_decl 0x7ffff6096f00 main>>
>>     supercontext <block 0x7ffff60c00f0 asm_written used
>>         vars <var_decl 0x7ffff608bb48 data type <record_type 0x7ffff608ec78
>> Data>
>>             used tree_1 tree_3 decl_5 SI file pr49115.C line 18 col 8
>>             size <integer_cst 0x7ffff5f42140 constant 32>
>>             unit size <integer_cst 0x7ffff5f42160 constant 4>
>>             align 128 context <function_decl 0x7ffff6096f00 main>
>>             (reg/v:SI 64 [ data ])>
>>         supercontext <block 0x7ffff5f4c550 asm_written used supercontext
>> <function_decl 0x7ffff6096f00 main>
>>             subblocks <block 0x7ffff5f4c500 asm_written used vars <var_decl
>> 0x7ffff608bb48 data> supercontext <block 0x7ffff5f4c550> subblocks <block
>> 0x7ffff5f4c4b0> chain <block 0x7ffff60c00f0>>>>>
>>
>> (gdb) p stmt->block
>> $2 = {base = {code = BLOCK, side_effects_flag = 0, constant_flag = 0,
>> addressable_flag = 0,
>>     volatile_flag = 0, readonly_flag = 0, asm_written_flag = 1, nowarning_flag
>> = 0, visited = 0,
>>     used_flag = 1, nothrow_flag = 0, static_flag = 0, public_flag = 0,
>> private_flag = 0,
>>     protected_flag = 0, deprecated_flag = 0, default_def_flag = 0, u = {bits =
>> {lang_flag_0 = 0,
>>         lang_flag_1 = 0, lang_flag_2 = 0, lang_flag_3 = 0, lang_flag_4 = 0,
>> lang_flag_5 = 0,
>>         lang_flag_6 = 0, saturating_flag = 0, unsigned_flag = 0, packed_flag =
>> 0, user_align = 0,
>>         nameless_flag = 1, spare0 = 0, spare1 = 0, address_space = 0}, length =
>> 2048,
>>       version = 2048}}, chain = 0x0, abstract_flag = 0, block_num = 14, locus =
>> 0,
>>   vars = 0x7ffff608bc78, nonlocalized_vars = 0x0, subblocks = 0x0, supercontext
>> = 0x7ffff60c00f0,
>>   abstract_origin = 0x0, fragment_origin = 0x0, fragment_chain = 0x0}
>>
>> Here's the fragment of assembly code between .LBB14 and .LBE14:
>>
>> .LBB14:
>>         # pr49115.C:21
>>         .loc 1 21 0
>>         call    __cxa_begin_catch
>> .LVL7:
>>         call    __cxa_end_catch
>> .LVL8:
>>         .p2align 4,,5
>> # SUCC: 3 [100.0%]  count:1
>>         jmp     .L15
>>         .cfi_endproc
>>         .section        .text.unlikely
>>         .cfi_startproc
>>         .cfi_personality 0x3,__gxx_personality_v0
>>         .cfi_lsda 0x3,.LLSDAC4
>> # BLOCK 6 freq:5000 seq:4
>> # PRED: 4 [50.0%]  (CROSSING,CAN_FALLTHRU)
>> .L14:
>>         .cfi_def_cfa_offset 16
>>         .p2align 4,,8
>> .LEHB1:
>> # SUCC:
>>         call    _Unwind_Resume
>> .LEHE1:
>> .LVL9:
>> .LBE14:
>> .LBE15:
>>         .cfi_endproc
>>
>> You can see that the block from .LBB14 to .LBE14 has been split across two
>> sections. In order for dwarf2out to generate the proper debug info,
>> BLOCK_FRAGMENT_CHAIN(stmt) needs to be non-NULL. I'm not sure why that's not
>> happening when the block is split.
>
> It looks like this is all done during the final pass when assembly is
> being emitted. In final_start_function the NOTE_INSN_BLOCK_{BEG/END}
> notes are inserted based on lexical scopes (in
> reemit_insn_block_notes). At the end of reemit_insn_block_notes,
> reorder_blocks is called to identify blocks references by more than
> one NOTE_INSN_BLOCK_{BEG/END}. Any such blocks are duplicated and the
> BLOCK_FRAGMENT_CHAIN is setup.
>
> I'm not familiar with this code, but perhaps when a switch section
> note is seen by reemit_insn_block_notes, it should invoke change_scope
> to emit the necessary NOTE_INSN_BLOCK_* notes?

Indeed the following patch fixes the problem. Cary, are you familiar
with this code and does it seem like a reasonable fix? If so I will
send to trunk for review.  It passed the gcc regression testing.

Index: final.c
===================================================================
--- final.c (revision 201461)
+++ final.c (working copy)
@@ -1560,6 +1560,16 @@ change_scope (rtx orig_insn, tree s1, tree s2)
   tree ts1 = s1, ts2 = s2;
   tree s;

+  if (NOTE_P (orig_insn) && NOTE_KIND (orig_insn) ==
NOTE_INSN_SWITCH_TEXT_SECTIONS)
+    {
+      gcc_assert (s1 == s1);
+      rtx note = emit_note_before (NOTE_INSN_BLOCK_END, orig_insn);
+      NOTE_BLOCK (note) = s1;
+      note = emit_note_before (NOTE_INSN_BLOCK_BEG, next_insn (orig_insn));
+      NOTE_BLOCK (note) = s1;
+      return;
+    }
+
   while (ts1 != ts2)
     {
       gcc_assert (ts1 && ts2);
@@ -1604,12 +1614,16 @@ reemit_insn_block_notes (void)
   rtx insn, note;

   insn = get_insns ();
-  if (!active_insn_p (insn))
-    insn = next_active_insn (insn);
-  for (; insn; insn = next_active_insn (insn))
+  for (; insn; insn = next_insn (insn))
     {
       tree this_block;

+      if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_SWITCH_TEXT_SECTIONS)
+        change_scope (insn, cur_block, cur_block);
+
+      if (!active_insn_p (insn))
+        continue;
+
       /* Avoid putting scope notes between jump table and its label.  */
       if (JUMP_TABLE_DATA_P (insn))
  continue;


Thanks,
Teresa

>
> Thanks,
> Teresa
>
>>
>> --
>> You are receiving this mail because:
>> You are on the CC list for the bug.
>> You reported the bug.
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

[Bug rtl-optimization/57451] Incorrect debug ranges emitted for -freorder-blocks-and-partition -g

[ccoutant at google dot com]

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

--- Comment #7 from ccoutant at google dot com ---
> Index: final.c
> ===================================================================
> --- final.c (revision 201461)
> +++ final.c (working copy)
> @@ -1560,6 +1560,16 @@ change_scope (rtx orig_insn, tree s1, tree s2)
>    tree ts1 = s1, ts2 = s2;
>    tree s;
>
> +  if (NOTE_P (orig_insn) && NOTE_KIND (orig_insn) ==
> NOTE_INSN_SWITCH_TEXT_SECTIONS)
> +    {
> +      gcc_assert (s1 == s1);

That should be s1 == s2, right?

> +      rtx note = emit_note_before (NOTE_INSN_BLOCK_END, orig_insn);
> +      NOTE_BLOCK (note) = s1;
> +      note = emit_note_before (NOTE_INSN_BLOCK_BEG, next_insn (orig_insn));
> +      NOTE_BLOCK (note) = s1;
> +      return;
> +    }
> +
>    while (ts1 != ts2)
>      {
>        gcc_assert (ts1 && ts2);
> @@ -1604,12 +1614,16 @@ reemit_insn_block_notes (void)
>    rtx insn, note;
>
>    insn = get_insns ();
> -  if (!active_insn_p (insn))
> -    insn = next_active_insn (insn);
> -  for (; insn; insn = next_active_insn (insn))
> +  for (; insn; insn = next_insn (insn))
>      {
>        tree this_block;
>
> +      if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_SWITCH_TEXT_SECTIONS)
> +        change_scope (insn, cur_block, cur_block);

It seems to me like you should just emit the three notes directly
here. This is really using change_scope for a new purpose, as you're
not really changing scopes, just putting a break into one.

> +      if (!active_insn_p (insn))
> +        continue;

I'm not clear on why this is needed. Is it because after the
change_scope, insn will now be a NOTE? If that's it, just put the
continue in the previous if clause.

-cary

[Bug rtl-optimization/57451] Incorrect debug ranges emitted for -freorder-blocks-and-partition -g

[tejohnson at google dot com]

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

--- Comment #8 from Teresa Johnson <tejohnson at google dot com> ---
On Fri, Aug 9, 2013 at 4:23 PM, ccoutant at google dot com
<gcc-bugzilla@gcc.gnu.org> wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57451
>
> --- Comment #7 from ccoutant at google dot com ---
>> Index: final.c
>> ===================================================================
>> --- final.c (revision 201461)
>> +++ final.c (working copy)
>> @@ -1560,6 +1560,16 @@ change_scope (rtx orig_insn, tree s1, tree s2)
>>    tree ts1 = s1, ts2 = s2;
>>    tree s;
>>
>> +  if (NOTE_P (orig_insn) && NOTE_KIND (orig_insn) ==
>> NOTE_INSN_SWITCH_TEXT_SECTIONS)
>> +    {
>> +      gcc_assert (s1 == s1);
>
> That should be s1 == s2, right?

Woops, yes.

>
>> +      rtx note = emit_note_before (NOTE_INSN_BLOCK_END, orig_insn);
>> +      NOTE_BLOCK (note) = s1;
>> +      note = emit_note_before (NOTE_INSN_BLOCK_BEG, next_insn (orig_insn));
>> +      NOTE_BLOCK (note) = s1;
>> +      return;
>> +    }
>> +
>>    while (ts1 != ts2)
>>      {
>>        gcc_assert (ts1 && ts2);
>> @@ -1604,12 +1614,16 @@ reemit_insn_block_notes (void)
>>    rtx insn, note;
>>
>>    insn = get_insns ();
>> -  if (!active_insn_p (insn))
>> -    insn = next_active_insn (insn);
>> -  for (; insn; insn = next_active_insn (insn))
>> +  for (; insn; insn = next_insn (insn))
>>      {
>>        tree this_block;
>>
>> +      if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_SWITCH_TEXT_SECTIONS)
>> +        change_scope (insn, cur_block, cur_block);
>
> It seems to me like you should just emit the three notes directly
> here. This is really using change_scope for a new purpose, as you're
> not really changing scopes, just putting a break into one.

Ok.

>
>> +      if (!active_insn_p (insn))
>> +        continue;
>
> I'm not clear on why this is needed. Is it because after the
> change_scope, insn will now be a NOTE? If that's it, just put the
> continue in the previous if clause.

Because the notes were being skipped by the iteration over
instructions, which previously only walked active instructions (notes
are not active instructions). So to see the switch section note I had
to walk all instructions, and just skip non-active instructions after
I am done checking for the note of interest.

Thanks,
Teresa

>
> -cary
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
> You reported the bug.

[Bug rtl-optimization/57451] Incorrect debug ranges emitted for -freorder-blocks-and-partition -g

[ccoutant at google dot com]

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

--- Comment #9 from ccoutant at google dot com ---
>>> +      if (!active_insn_p (insn))
>>> +        continue;
>>
>> I'm not clear on why this is needed. Is it because after the
>> change_scope, insn will now be a NOTE? If that's it, just put the
>> continue in the previous if clause.
>
> Because the notes were being skipped by the iteration over
> instructions, which previously only walked active instructions (notes
> are not active instructions). So to see the switch section note I had
> to walk all instructions, and just skip non-active instructions after
> I am done checking for the note of interest.

Oh, right. I didn't notice the change in the for loop.

-cary

[Bug rtl-optimization/57451] Incorrect debug ranges emitted for -freorder-blocks-and-partition -g

[tejohnson at gcc dot gnu.org]

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

--- Comment #10 from tejohnson at gcc dot gnu.org ---
Author: tejohnson
Date: Sat Nov 23 04:30:07 2013
New Revision: 205298

URL: http://gcc.gnu.org/viewcvs?rev=205298&root=gcc&view=rev
Log:
Backport to google/4_8 new sanity checking and some dwarf emission fixes for
-freorder-blocks-and-partition from trunk (r201883, r201941, r202125).

------------------------------------------------------------------------
r201883 | tejohnson | 2013-08-20 06:29:53 -0700 (Tue, 20 Aug 2013) | 8 lines
Changed paths:
   M /trunk/gcc/ChangeLog
   M /trunk/gcc/final.c
   M /trunk/gcc/testsuite/ChangeLog
   A /trunk/gcc/testsuite/g++.dg/tree-prof/pr57451.C

2013-08-20  Teresa Johnson  <tejohnson@google.com>

    PR rtl-optimizations/57451
    * final.c (reemit_insn_block_notes): Prevent lexical blocks
    from crossing split section boundaries.

    * testsuite/g++.dg/tree-prof/pr57451.C: New test.

------------------------------------------------------------------------
------------------------------------------------------------------------
r201941 | tejohnson | 2013-08-23 07:31:06 -0700 (Fri, 23 Aug 2013) | 7 lines
Changed paths:
   M /trunk/gcc/ChangeLog
   M /trunk/gcc/final.c

2013-08-23  Kaz Kojima  <kkojima@gcc.gnu.org>

        PR rtl-optimization/58220
        PR regression/58221
    * final.c (reemit_insn_block_notes): Use NEXT_INSN to
        handle SEQUENCE insns properly.

------------------------------------------------------------------------
------------------------------------------------------------------------
r202125 | tejohnson | 2013-08-30 18:43:33 -0700 (Fri, 30 Aug 2013) | 30 lines
Changed paths:
   M /trunk/gcc/ChangeLog
   M /trunk/gcc/basic-block.h
   M /trunk/gcc/bb-reorder.c
   M /trunk/gcc/cfg.c
   M /trunk/gcc/cfgcleanup.c
   M /trunk/gcc/cfgrtl.c
   M /trunk/gcc/predict.c

This patch sanitizes the partitioning to address issues such as edge
weight insanities that sometimes occur due to upstream optimizations,
and ensures that hot blocks are not dominated by cold blocks. This
needs to be resanitized after certain cfg optimizations that may
cause hot blocks previously reached via both hot and cold paths to
only be reached by cold paths.

The verification code in sanitize_dominator_hotness was contributed by
Steven Bosscher.

2013-08-29  Teresa Johnson  <tejohnson@google.com>
            Steven Bosscher  <steven@gcc.gnu.org>

    * cfgrtl.c (fixup_new_cold_bb): New routine.
    (commit_edge_insertions): Invoke fixup_partitions.
    (find_partition_fixes): New routine.
    (fixup_partitions): Ditto.
    (verify_hot_cold_block_grouping): Update comments.
    (rtl_verify_edges): Invoke find_partition_fixes.
    (rtl_verify_bb_pointers): Update comments.
    (rtl_verify_bb_layout): Ditto.
    * basic-block.h (probably_never_executed_edge_p): Declare.
        (fixup_partitions): Ditto.
    * cfgcleanup.c (try_optimize_cfg): Invoke fixup_partitions.
    * bb-reorder.c (sanitize_hot_paths): New function.
        (find_rarely_executed_basic_blocks_and_crossing_edges): Invoke
        sanitize_hot_paths.
    * predict.c (probably_never_executed_edge_p): New routine.
    * cfg.c (check_bb_profile): Add partition insanity warnings.

------------------------------------------------------------------------

Added:
    branches/google/gcc-4_8/gcc/testsuite/g++.dg/tree-prof/pr57451.C
Modified:
    branches/google/gcc-4_8/gcc/basic-block.h
    branches/google/gcc-4_8/gcc/bb-reorder.c
    branches/google/gcc-4_8/gcc/cfg.c
    branches/google/gcc-4_8/gcc/cfgcleanup.c
    branches/google/gcc-4_8/gcc/cfgrtl.c
    branches/google/gcc-4_8/gcc/final.c
    branches/google/gcc-4_8/gcc/predict.c