<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 18, 2015 at 7:47 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">chandlerc added a comment.<br>
<span><br>
In <a href="http://reviews.llvm.org/D15401#314253" rel="noreferrer" target="_blank">http://reviews.llvm.org/D15401#314253</a>, @chandlerc wrote:<br>
<br>
> In <a href="http://reviews.llvm.org/D15401#312525" rel="noreferrer" target="_blank">http://reviews.llvm.org/D15401#312525</a>, @eraman wrote:<br>
><br>
> > > 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.<br>
> ><br>
> ><br>
> > 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<br>
><br>
><br>
> I had to spend a bunch of time thinking about this.<br>
><br>
> 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.<br>
><br>
> 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.<br>
<br>
<br>
</span>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.<br>
<br>
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.<br>
<div><div><br></div></div></blockquote><div>I agree that it doesn't make sense to have ICA as an analysis pass and have sent a refactoring patch.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D15401" rel="noreferrer" target="_blank">http://reviews.llvm.org/D15401</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div></div>