SampleProfileLoader inlines hot functions if it is inlined in the profiled binary. However, the inline needs to be guarded by legality check, otherwise it could lead to correctness issues.
- rG3a81f84d9aae: Invoke GetInlineCost for legality check before inline functions in…
rL313277: Invoke GetInlineCost for legality check before inline functions in…
rL313195: Invoke GetInlineCost for legality check before inline functions in…
rG15c86ef970dc: Invoke GetInlineCost for legality check before inline functions in…
Do you really need an option for this? All you want is to pick a large enough number such that the cost never exceeds the threshold.
This needs detailed comments. State that you want to check if there is anything in the reachable portion of the callee at this callsite that makes this inlining potentially illegal and that's why you use a large threshold (large enough to return true in ~all cases except when it is potentially incorrect)
Differentiate the InlineCost::isNever and the case where cost exceeds threshold. It is useful to log the latter case for debugging.
Discussed this offline with Dehao. Instead of using an arbitrary threshold of 100000, it is better to piggyback on the the option in InlineCost to compute cost analysis in full. That way, if the analysis returns NeverInlineCost, Sample loader shouldn't inline. Anything else (doesn't matter if the cost >= threshold), it can do the inlining. I will refactor InlineCost.cpp to enable the feature to be used without requiring an option and Dehao will base this patch on top of that.
There is a uninitialized value, so I've reverted the patch with r313230 to fix bots.