Re: [PATCH] Disable -fno-reorder-blocks-and-partition if no -fprofile-use to avoid unnecessary overhead

[Teresa Johnson <tejohnson@google.com>]

On Fri, Sep 25, 2015 at 6:58 AM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 09/24/2015 07:16 PM, Teresa Johnson wrote:
>>
>> This patch unsets -freorder-blocks-and-partition when -fprofile-use
>> is not specified. Function splitting was not actually being performed
>> in that case, as probably_never_executed_bb_p does not distinguish
>> any basic blocks as being cold vs hot when there is no profile data.
>>
>> Leaving it enabled, however, causes the assembly code generator to create
>> (empty) cold sections and labels, leading to unnecessary size overhead.
>>
>> Bootstrapped and tested on x86-64-unknown-linux-gnu. Ok for trunk?
>>
>> Thanks,
>> Teresa
>>
>> 2015-09-24  Teresa Johnson  <tejohnson@google.com>
>>
>>          * opts.c (finish_options): Unset -freorder-blocks-and-partition
>>          if not using profile.
>
>
> Hmm, I'd noticed I was getting that enabled by default. It looks like you
> added this default with:
>
> 2013-11-19  Teresa Johnson  <tejohnson@google.com>
>
>        * common/config/i386/i386-common.c: Enable
>        -freorder-blocks-and-partition at -O2 and up for x86.
>        * doc/invoke.texi: Update -freorder-blocks-and-partition default.
>        * opts.c (finish_options): Only warn if -freorder-blocks-and-
>        partition was set on command line.
>
> (Note that this ChangeLog entry should have mentioned
> ix86_option_optimization_table as the variable that was changed).

Yeah, looks like I accidentally left the function name out of the
i386-common.c entry. I can fix the ChangeLog entry.

>
> What has changed between then and now? Also, do we not use
> estimates/heuristics when not using a profile?

Nothing has changed - splitting effectively never kicked in without a
profile. Honza and I had discussed the idea that static profile
heuristics could eventually be used to distinguish hot vs cold bbs,
but that hasn't happened. What I didn't notice until recently was the
size increase in the .o files from varasm adding in unnecessary
sections and labels when this option was on. Unless and until static
heuristics are used to distinguish cold bbs in
probably_never_executed_bb_p, I don't think it makes sense to do
anything finer grained that just disabling the option.

Teresa

>
>
> Bernd



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