[PATCH] D15401: Refactor threshold computation for inline cost analysis

[Easwaran Raman via llvm-commits]

On Fri, Dec 18, 2015 at 7:47 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> chandlerc added a comment.
>
> In http://reviews.llvm.org/D15401#314253, @chandlerc wrote:
>
> > In http://reviews.llvm.org/D15401#312525, @eraman wrote:
> >
> > > > I'm just suggesting changing the *APIs* to deal in symbolic
> settings. Clearly the flag will be numeric. I'm suggested sinking the
> inline threshold flag into the inline cost analysis completely.
> > >
> > >
> > > I attempted to do that and hit an issue. InlineSimple.cpp provides a
> createFunctionInliningPass((int Threshold) API. To sink thresholds to
> InlineCost, this needs to be removed, but this is called by
> LLVMPassManagerBuilderUseInlinerWithThreshold which is exposed by the
> llvm-c API
> >
> >
> > I had to spend a bunch of time thinking about this.
> >
> > On one hand, I think exposing this kind of configurability is really
> frustrating from an API-design perspective. But I think I can imagine users
> of LLVM (particularly library users) wanting to have pretty wildly
> different inlining tolerances. However, this raises an important question
> of how that should be propagated to when the cost analysis has to recurse
> across yet another function call.
> >
> > I think we need to move *completely* away from having different
> *initial* thresholds for things like inline-hint and opt-size or min-size.
> We have numerous adjustments to the threshold based on different analyses
> properties. I think inline-hint and size based stuff should work the same
> way. This will let you just sink the capping and ballooning of the
> threshold into analyzeCall where we also compute all the bonuses. Does that
> make sense? It should also avoid the need to separately call
> 'getInlineThreshold' -- you'll just store and pass along the initial
> threshold.
>
>
> One thing that may make it much easier to innact this refactoring in a
> simple way would be to first do another much over-due refactoring to make
> InlineCostAnalysis not *actually* be an analysis *pass*. The only
> "analysis" done is to capture two pointers. Instead, this should just be a
> utility class and method that the inliner pass constructs (passing in the
> relevant bits) and uses.
>
> If it would be helpful, I can send you a patch that does that, but I don't
> want to make merging and such more complicated if it makes more sense for
> you to do this on your end.
>
> I agree that it doesn't make sense to have ICA as an analysis pass and
have sent a refactoring patch.


>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D15401
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151221/418519f7/attachment.html>