[PATCH] D15245: Use a higher inlining threshold for hot callees.

[Xinliang David Li via llvm-commits]

On Fri, Dec 4, 2015 at 5:33 PM, Easwaran Raman <eraman at google.com> wrote:
>
>
> On Fri, Dec 4, 2015 at 4:13 PM, Justin Bogner <mail at justinbogner.com> wrote:
>>
>> Easwaran Raman <eraman at google.com> writes:
>> > eraman created this revision.
>> > eraman added reviewers: davidxl, bogner.
>> > eraman added subscribers: dnovillo, llvm-commits.
>> > eraman set the repository for this revision to rL LLVM.
>> >
>> > This patch uses the higher inline-hint-threshold for callees that are
>> > considered hot. The threshold for hotness is the one used in CFE to
>> > apply inline hint on hot callees - callee's entry count >= 30% of max
>> > function entry count. The idea is to remove that code from the
>> > frontend and make the PGO based inlining decisions in the inliner
>> > itself. What we really want is callsite based hotness thresholds, but
>> > that is a heuristic change that needs tuning and will be done in a
>> > separate patch.
>> >
>> > Repository:
>> >   rL LLVM
>> >
>> > http://reviews.llvm.org/D15245
>> >
>> > Files:
>> >   lib/Transforms/IPO/Inliner.cpp
>> >
>> > Index: lib/Transforms/IPO/Inliner.cpp
>> > ===================================================================
>> > --- lib/Transforms/IPO/Inliner.cpp
>> > +++ lib/Transforms/IPO/Inliner.cpp
>> > @@ -296,6 +296,17 @@
>> >    if (InlineHint && HintThreshold > Threshold &&
>> >        !Caller->hasFnAttribute(Attribute::MinSize))
>> >      Threshold = HintThreshold;
>> > +  // If profile information is available, use that to prioritize hot
>> > functions.
>> > +  // FIXME: The heuristic used here is based on preliminary SPEC tuning
>> > and
>> > +  // may not be optimal. Replace this with a well-tuned heuristic based
>> > on
>> > +  // *callsite* hotness and not callee hotness.
>> > +  auto EntryCount = Callee->getEntryCount();
>> > +  auto MaxFunctionCount =
>> > Callee->getParent()->getMaximumFunctionCount();
>> > +  if (EntryCount && MaxFunctionCount &&
>> > +      EntryCount.getValue() >=
>> > +          (uint64_t)(0.3 * (double)MaxFunctionCount.getValue())) {
>> > +    Threshold = HintThreshold;
>> > +  }
>>
>> Moving this out of the frontend to here makes sense, but I have a couple
>> of questions.
>>
>> - Does this only apply when we've supplied actual profile data, or can
>>   we hit it based off of the heuristic counts in no-profile-data mode?
>>   If the latter, this isn't quite NFC.
>>
> It applies only when there is profile data since entry count and maximum
> function count come only from profile data.
>
>>
>> - Can the same be done for the "cold" attribute the frontend also applies?
>>
> The difference between the inlinehint and cold attributes is that the latter
> is used not just by the inliner. For instance, branch probability info. I
> can move it to the backend and replace all calls to
> hasFnAttr(Attribute::Cold) with a wrapper that also returns true if the
> count is below the threshold. Do you want that change to be in this patch?

Removing Cold Attribute from FE is a good thing to do too -- I won't
mind it be done in a seperate patch.

David



>
> regards,
> Easwaran
>
>>
>> >
>> >    // Listen to the cold attribute when it would decrease the threshold.
>> >    bool ColdCallee = Callee && !Callee->isDeclaration() &&
>>
>>
>