[PATCH] D34471: [Inliner] Boost inlining of an indirect call to always_inline function.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

In https://reviews.llvm.org/D34471#788736, @davidxl wrote:

> Chandler's example does not involve any indirect calls so it is not related to this patch.


Actually, it is...

The original test case doesn't look exactly like my example. Rather than a template argument and a call through that template argument, it uses an indirect call to a normal function argument.

My suggestion is that the code should be changed to use a template argument if the intent is to make `always_inline` actually *always* inline.

> I think Easwaran's patch is consistent with the current inline behavior -- it boots inlining to callee with indirect callsites that can be turned into direct calls and then further inlined. The amount of boost is the benefit of inlining the indirect callsites (i.e., the difference between inline cost and allowed threshold). In fact, if the CA.analyzeCall call in the existing code
> 
>   auto IndirectCallParams = Params;
>   IndirectCallParams.DefaultThreshold = InlineConstants::IndirectCallThreshold;
>   CallAnalyzer CA(TTI, GetAssumptionCache, GetBFI, PSI, *F, CS,
>                   IndirectCallParams);
>   if (CA.analyzeCall(CS)) {
>       ...
>     
> 
> is turned into getInlineCost(CS), it will achieve similar effect -- except that InlineCost associated with AlwaysInline is a little too extreme.
> 
> In many cases, AlwaysInline is very strong hint by the user to indicate performance benefit of inlining the function, it does not make sense to completely ignore it.

I don't think this is really the right way to model the `always_inline` attribute. I continue to think that this attribute should mean that we very literally *always* inline. See the discussion here for more details:
http://lists.llvm.org/pipermail/llvm-dev/2015-August/089466.html

(Note that I *also* am still in favor of having an attribute that is actually a very strong hint to do inlining, I just don't think any of the existing attributes really fit that bill, and I don't think the original motivating test case is a particularly compelling example. Even if a user *does* want to get really strong inlining because of a weird indirect function call, I don't know why the inliner has to go to great lengths to detect this rather than expecting the user to also attribute the function receiving the function pointer...)


https://reviews.llvm.org/D34471