[PATCH] D15701: Refactor inline costs analysis by removing the InlineCostAnalysis class

[Easwaran Raman via llvm-commits]

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.


Ok.

> 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.


Sure.


Repository:
  rL LLVM

http://reviews.llvm.org/D15701