[llvm] r200886 - Inliner uses a smaller inline threshold for callees with cold attribute.

[Manman Ren]

On Wed, Feb 5, 2014 at 4:34 PM, Chandler Carruth <chandlerc at google.com>wrote:

>
> On Wed, Feb 5, 2014 at 3:58 PM, Manman Ren <manman.ren at gmail.com> wrote:
>
>> I thought it is straight-forward to use a lower threshold for cold
>> functions and didn't think this patch is going to have a wide & dramatic
>> impact.
>>
>> I am not aware of any benchmark in lnt that uses cold attribute. If there
>> is one, please let me know so I can collect some performance number.
>>
>
> Surely there are benchmarks with more representative code than SPEC? Maybe
> not. There is also no cold attribute in the test suite.
>
>
>>
>> When Diego first introduced the Cold attribute and used it to adjust
>> block weights, there was no request on benchmark numbers. So I assumed
>> negligible performance impact for this patch.
>>
>
> Sure, but there wasn't any use of cold in LLVM before. Also, changing
> inliner heuristics has an *incredibly* broader impact. It literally has the
> largest impact in the entire compiler.
>

Agree.

>
> Notably, his change didn't introduce any particularly interesting
> thresholds. It reused existing thresholds for __builtin_expect IIRC.
>
> There at least needs to be *some* science behind the use of 75 for the
> magical threshold here. When setting the other thresholds both Jakob and I
> would run large benchmark suites across a wide range of values and try to
> dig out the shape of the graph.
>

When setting threshold for inlinehint, do you recall what benchmark suites
we collected data on?


>
>>
>> Sorry if this caused regressions. The main purpose of this is performance
>> of instrumentation based PGO.
>>
>
> I understand that this is the intent, but the whole point of PGO just
> feeding the existing ways to represent things in the IR is because we
> already use those when we *don't* have a profile, and any optimizations
> will apply to both.
>
>
> I'd like to see some numbers behind the use of "75" in the inliner.
>

75 is the threshold we use when optimizing for size (OptSizeThreshold). I
used the same number for cold functions.


> Until then, I'd ask to keep the actual threshold unchanged and just
> introduce the flag so that folks can conduct experiments to collect this
> data. Then you can send a request to the mailing list that folks conduct
> such experiments and collect feedback from the community. This pattern has
> worked extremely well recently as Arnold has added new features and tuned
> thresholds in the vectorizer. I think the inliner deserves the same or more
> scrutiny.
>

Sure. I will set the default inlinecold-threshold to be the same as
inline-threshold (225). About conducting experiments, without PGO setting
cold attribute, we will need benchmarks that actually use "cold" attribute.

Thanks,
Manman
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140205/7b7fc445/attachment.html>