[PATCH] Fix PR 53743 and other -freorder-blocks-and-partition failures (issue6823047)

[Teresa Johnson]

On Tue, Oct 30, 2012 at 2:33 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Tue, Oct 30, 2012 at 10:28 PM, Steven Bosscher wrote:
>> Hello Teresa,
>>
>> Could you try this patch for me also? It moves bbpart outside the part
>> of the passes pipeline that works in cfglayout mode.
>
> where's the "unsend" button if you need it...
>
> So, to complete the mail...
>
> Could you try this patch for me also? It moves bbpart outside the part
> of the passes pipeline that works in cfglayout mode. It looks like
> when someone (/me looks the other way) changed the compiler to work
> like that, he/she forgot about updating this pass...

Sure, I will give this a try after your verification patch tests
complete. Does this mean that the patch you posted above to
force_nonfallthru_and_redirect is no longer needed either? I'll see if
I can avoid the need for some of my fixes, although I believe at least
the function.c one will still be needed. I'll check.

Regarding your earlier question about why we needed to add the
barrier, I need to dig up the details again but essentially I found
that the barriers were being added by bbpart, but bbro was reordering
things and the block that ended up at the border between the hot and
cold section didn't necessarily have a barrier on it because it was
not previously at the region boundary.


>
> Instead, the pass should run just before register allocation, as late
> as possible so that any funny CFG modifications have taken place. A
> possible down-side is that profile info may have degenerated a bit
> further, but I don't think that's a serious concern because the
> partitioning is actually quite stupid: Just stuff all blocks with
> count==0 into the cold section. Updating 0-counts is easy enough that
> I think GCC should get that right everywhere.
>
> It'd be nice to figure out if there are less stupid^Wsimplistic
> heuristics to decide what should go into the cold partition, for GCC
> 4.9...

Yep, that is one of my goals in looking at this - I am hoping to use
the new fdo summary info I added to tune this into a more robust
decision based on the counter values in the working set summary.

Thanks for the help!
Teresa

>
> Ciao!
> Steven
>
>
>         * passes.c (init_optimization_passes): Move
> pass_partition_blocks just before RA.
>
> Index: passes.c
> ===================================================================
> --- passes.c    (revision 192995)
> +++ passes.c    (working copy)
> @@ -1595,7 +1595,6 @@ init_optimization_passes (void)
>        NEXT_PASS (pass_ud_rtl_dce);
>        NEXT_PASS (pass_combine);
>        NEXT_PASS (pass_if_after_combine);
> -      NEXT_PASS (pass_partition_blocks);
>        NEXT_PASS (pass_regmove);
>        NEXT_PASS (pass_outof_cfg_layout_mode);
>        NEXT_PASS (pass_split_all_insns);
> @@ -1606,6 +1605,7 @@ init_optimization_passes (void)
>        NEXT_PASS (pass_match_asm_constraints);
>        NEXT_PASS (pass_sms);
>        NEXT_PASS (pass_sched);
> +      NEXT_PASS (pass_partition_blocks);
>        NEXT_PASS (pass_ira);
>        NEXT_PASS (pass_reload);
>        NEXT_PASS (pass_postreload);



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