Page MenuHomePhabricator

[InlineCost] Small changes to early exit condition. NFC.
ClosedPublic

Authored by haicheng on Aug 23 2017, 4:40 PM.

Details

Summary

If Cost < Threshold, we will try to inline the callee. So, we can early exit when Cost >= Threshold which is earlier than Cost > Threshold.

Diff Detail

Repository
rL LLVM

Event Timeline

haicheng created this revision.Aug 23 2017, 4:40 PM
eastig added a subscriber: eastig.Aug 24 2017, 1:53 AM
eastig added inline comments.Aug 24 2017, 3:34 AM
lib/Analysis/InlineCost.cpp
1209 ↗(On Diff #112481)

I don't think this is NFC. In visitSwitchInst I see the cost is calculating based on a result of TTI.getEstimatedNumberOfCaseClusters. If threshold is big (-O1 and above) this might not be a problem but if it is low (-Os, -Oz) this might be a problem.

haicheng updated this revision to Diff 112568.Aug 24 2017, 8:37 AM

Keep the check Evgeny has question about unchanged.

junbuml added inline comments.Aug 24 2017, 9:18 AM
lib/Analysis/InlineCost.cpp
1209 ↗(On Diff #112481)

Specifically for this part, we cannot say that this is NFC in case when "CostLowerBound == Threshold" and the switch ends up with a bit test. See above "FIXME". However, I cannot see any big problem using ">=" here.

eastig accepted this revision.Aug 25 2017, 1:58 AM

LGTM
Thank you, Haicheng.

This revision is now accepted and ready to land.Aug 25 2017, 1:58 AM
eastig added inline comments.Aug 25 2017, 2:01 AM
lib/Analysis/InlineCost.cpp
1209 ↗(On Diff #112481)

I agree it does not look like a big problem but to be sure some benchmarking, especially code size, needs to be done.

This revision was automatically updated to reflect the committed changes.