This is an archive of the discontinued LLVM Phabricator instance.

[InlineCost] Add cl::opt to allow full inline cost to be computed for debugging purposes.
ClosedPublic

Authored by haicheng on Jul 25 2017, 11:58 AM.

Details

Summary

Currently, the inline cost model will bail once the inline cost exceeds the inline threshold in order to avoid unnecessary compile-time. However, when debugging I found it useful to compute the full cost, so I added this command line option to override the default behavior.

I wanted to see if others thought this might be useful. I also wanted to have someone more familiar with the inline cost model double check to make sure the patch is correct.

Chad

Diff Detail

Repository
rL LLVM

Event Timeline

mcrosier created this revision.Jul 25 2017, 11:58 AM
haicheng added inline comments.
lib/Analysis/InlineCost.cpp
1070 ↗(On Diff #108131)

I think you need to skip this, too.

mcrosier added inline comments.Jul 25 2017, 12:28 PM
lib/Analysis/InlineCost.cpp
1070 ↗(On Diff #108131)

I believe you're correct. Thanks, Haicheng.

mcrosier abandoned this revision.Aug 2 2017, 3:31 PM
chandlerc edited edge metadata.Aug 2 2017, 4:10 PM

Why abandon? FWIW, this does seem really useful. I'd also suggest wiring it up so that when optimization remarks are enabled, we do the extra compile time work to give that a detailed answer. (Or maybe set a *really high* threshold below which we give a reasonably exact answer...)

Why abandon? FWIW, this does seem really useful. I'd also suggest wiring it up so that when optimization remarks are enabled, we do the extra compile time work to give that a detailed answer. (Or maybe set a *really high* threshold below which we give a reasonably exact answer...)

I just didn't think there was any interest, but if you think it might be useful, I'll revive the patch.

mcrosier reclaimed this revision.Aug 9 2017, 6:38 AM

@haicheng has offered to take up this work!

haicheng commandeered this revision.Aug 9 2017, 6:40 AM
haicheng added a reviewer: mcrosier.

Take the work from Chad.

haicheng updated this revision to Diff 110380.Aug 9 2017, 6:43 AM

Wire up with Optimization Remarks.

haicheng marked an inline comment as done.Aug 9 2017, 6:43 AM
eraman edited edge metadata.Aug 9 2017, 11:29 AM

analyzeBlock bails out when inlining is not feasible due to things like recursive call, dynamic alloca etc. Arguably it doesn't make sense to compute the full inline cost even with this new option (and that's the behavior with this patch). In that case, especially with the ORE hookup, there should be some indication that the cost is not fully computed because the callee can not get inlined even if cost < threshold.

analyzeBlock bails out when inlining is not feasible due to things like recursive call, dynamic alloca etc. Arguably it doesn't make sense to compute the full inline cost even with this new option (and that's the behavior with this patch). In that case, especially with the ORE hookup, there should be some indication that the cost is not fully computed because the callee can not get inlined even if cost < threshold.

With option -inline-cost-full enabled, the output of optimization remarks is like below if anything like recursive call, dynamic alloca is found.

foz not inlined into bar because it should never be inlined (cost=never)

Pelease let me know how do you want it to be changed.

Haicheng

haicheng updated this revision to Diff 111454.EditedAug 16 2017, 6:57 PM

Change the output of optimization remarks to something like below if anything like recursive call, dynamic alloca is found

foz is recursive and allocates too much stack space. Cost is not fully computed
foz not inlined into bar because it should never be inlined (cost=never)

Is this okay? Thank you.

Change the output of optimization remarks to something like below if anything like recursive call, dynamic alloca is found

foz is recursive and allocates too much stack space. Cost is not fully computed
foz not inlined into bar because it should never be inlined (cost=never)

Is this okay? Thank you.

Yes, that looks good. Even just a "Cost is not fully computed" is fine.

mcrosier added inline comments.Aug 18 2017, 9:21 AM
lib/Analysis/InlineCost.cpp
1366 ↗(On Diff #111454)

unilinable -> uninlinable

haicheng updated this revision to Diff 111918.Aug 20 2017, 5:52 PM

Fixed a typo found by Chad. Thank you.

mcrosier accepted this revision.Aug 21 2017, 9:23 AM

LGTM.

This revision is now accepted and ready to land.Aug 21 2017, 9:23 AM
This revision was automatically updated to reflect the committed changes.