[PATCH] D36976: [Inliner] Tweak the condition for determining cold callsites.

Easwaran Raman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 25 14:30:02 PDT 2017


eraman added a comment.

In https://reviews.llvm.org/D36976#850603, @eraman wrote:

> In https://reviews.llvm.org/D36976#850579, @chandlerc wrote:
>
> > In https://reviews.llvm.org/D36976#850558, @eraman wrote:
> >
> > > Thinking about this a little more, this will end up treating non-cold callsites as cold. As an exmple, consider bar gets inlined to foo. Let entry_freq(foo) = 8, entry_freq(bar) = 8, callsite_freq(foo->bar) = 7. Now, any block in bar with freq == 1 will end up with a frequency of 0 after cloning and will be treated as cold even it's frequency relative to foo's entry is 1/7 which is not cold for the default value of cold-callsite-rel-freq (1/50). Rouding the freuenies while scaling will mitigate this a little bit, but still not sufficient. Need to think more.
> >
> >
> > I think you'll need to saturate at 1, and if necessary to preserve precision scale the caller up.
>
>
> Scaling the caller up pretty much defeats the purpose of incremental BFI updates and we no longer have the property that cost of a single inlining is constant.


We only need the block frequencies of blocks containing callsites. We already maintain the list of callsites including those obtained by inlining. We could keep track of the relative (to the entry of the caller) block frequencies  as a ScaledNumber and use this in InlineCost and in InlineFunction.cpp (to update the profile counts). Thoughts?


https://reviews.llvm.org/D36976





More information about the llvm-commits mailing list