[PATCH] D107897: [FuncSpec] Don't specialize function which are easy to inline

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

In D107897#2939299 <https://reviews.llvm.org/D107897#2939299>, @SjoerdMeijer wrote:

>> For spec2017int, this patch could reduce the number of specialized functions by 33%. Then the compile time didn't increase for every benchmark.
>
> Do you mean that with this new default of 100, we see less function specialising for SPEC2017?

Yes. And the improved benchmarks could still get the improvement.

In D107897#2939910 <https://reviews.llvm.org/D107897#2939910>, @snehasish wrote:

> I think this patch raises the question whether the pass today is running at the right place in the pipeline with respect to the inliner. To me it seems that either the cost modeling should leverage the InlineCost heuristics (rather than arbitrary constants tuned on SPEC) or specialize post-inlining where such decision-making may no longer be necessary. This is particularly important with the presence of PGO information which significantly affects inlining decisions. It would be appreciated if we could spend some time studying the impact of timing of the pass before committing to this direction.

Yes, this is the key question. Let me elaborate it more. The reason why we put this in the front of the inliner is that we could find opportunities to covert indirect call to direct call by function specialization. Then the inliner could make big benefit for that. And if we put function specialization after the inliner, we could only get the profit for constant passing.
BTW it looks like a common opportunity for me to convert indirect call to direct call since there are many functions being passed in actual codes. (Although the implementation for function specialization don't scale to be so powerful, that's our future direction).
So here is the decision point:

- Put function specialization in the front of the inliner.
  - We have the chance to convert indirect call to direct call.
  - There may be unuseful specialization which would be covered by the inliner. And this is what this patch tries to mitigate.
- Put function specialization in the back of the inliner.
  - We would miss the opportunity to convert indirect call to direct call.
  - We could avoid unuseful specialization totally.

>From my perspective, it looks **not bad** after we add heuristics (no-matter simple constant or inline cost) to avoid specializing functions which would be inlined finally. Since we get the optimization opportunity and avoid many unused specialization.

For this topic about pass order, I have a more crazy idea:
inliner --> function specializer --> inliner (may be a new inliner pass)
To avoid wasting the time, we could add special attribute in function specializer. And let the second inliner to decide whether or not to inline these functions. The second inliner is also responsible to remove the attributes.

But the question why I didn't implement this is : is it really much better than this simple patch? On the one hand, we need write much more codes and it means more compile time generally. On the other hand, would the generated code be so different?
The reason I prefer this patch is the simplicity and effectiveness in a degree.

> To me it seems that either the cost modeling should leverage the InlineCost heuristics (rather than arbitrary constants tuned on SPEC)

The InlineCost heuristics is for callsites instead of functions. If we decide to use InlineCost as a filter, we need run InlineCost for every callsite with constant parameter. It looks expensive to me since the calculation of InlineCost is not simple actually. And the estimation for functions is much cheaper.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107897/new/

https://reviews.llvm.org/D107897