[PATCH] D15701: Refactor inline costs analysis by removing the InlineCostAnalysis class
Easwaran Raman via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 28 12:21:54 PST 2015
eraman added a comment.
In http://reviews.llvm.org/D15701#317434, @chandlerc wrote:
> This LGTM, feel free to submit with the argument naming tweak below.
> However, this naming tweak raises an interesting question for me. This should *not* be addressed in this patch, but it might be a good thing to put on your queue.
> We are passing the *callee* TTI into the inline cost analysis. That doesn't make a lot of sense to me. We're inlining into the *caller*. If there is something *incompatible* about the callee, we should refuse to inline it. If they are compatible, the *caller's* TTI should become dominant. As an example, if we have one function which is marked as optimized for size, but we inline it into code that is not optimized for size, I would expect the inlined body to *not* be optimized for size. I would generally expect the TTI of the call site to determine the cost analysis for inlining because after inlining, the callee is gone. Does that make sense?
Yes. We will have to pass both the caller and callee TTIs when I get to implementing estimated speedup that requires computing the cost of the inlined and uninlined versions of the callee.
> That will be a non-trivial functional change, so you'll want to benchmark it etc before making it.
> Relatedly, I know you're looking heavily at getting our nested call site analysis to be accurate. You should make sure we're considering the possibility of incompatible function attributes that might prevent inlining entirely. I don't think we currently model that correctly, in that the cost analysis skips it but then the inliner itself considers it.
More information about the llvm-commits