Page MenuHomePhabricator

[CSSPGO] Factor out common part for CSSPGO inline and AFDO inline
ClosedPublic

Authored by wenlei on Jan 19 2021, 11:54 PM.

Details

Summary

Refactoring SampleProfileLoader::inlineHotFunctions to use helpers from CSSPGO inlining and reduce similar code in the inlining loop, plus minor cleanup for AFDO path.

Test Plan:

Diff Detail

Event Timeline

wenlei created this revision.Jan 19 2021, 11:54 PM
wenlei requested review of this revision.Jan 19 2021, 11:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2021, 11:54 PM
wenlei added inline comments.Jan 20 2021, 12:12 AM
llvm/lib/Transforms/IPO/SampleProfile.cpp
1419

In the old AFDO inliner, cost-benefit check is done earlier, so at this point we would skip inlining only if cost is never. When the cost is not never, the cost is only used in remarks. I figured a remark with positive inlining and cost > threshold in the message can be confusing, so I tweak the threshold here. This is why I had to update the test case.

wmi accepted this revision.Jan 21 2021, 1:23 PM

That is a very helpful refactoring. Thanks!

llvm/lib/Transforms/IPO/SampleProfile.cpp
419–423

Maybe have a comment explaining the params, especially showing some of them are input and some are output.

This revision is now accepted and ready to land.Jan 21 2021, 1:23 PM
hoy added a comment.Jan 22 2021, 2:48 PM

LGTM, thanks for the refactoring.

hoy accepted this revision.Jan 22 2021, 2:48 PM
wenlei updated this revision to Diff 318759.Jan 23 2021, 8:32 AM

Update comment based on Wei's feedback.

wenlei added inline comments.Jan 23 2021, 8:33 AM
llvm/lib/Transforms/IPO/SampleProfile.cpp
419–423

Comments added to definition.

wenlei marked an inline comment as done.Jan 23 2021, 8:33 AM
wenlei retitled this revision from [NFC] Factor out common part for CSSPGO inline and AFDO inline to [CSSPGO] Factor out common part for CSSPGO inline and AFDO inline.Jan 26 2021, 8:20 AM
This revision was landed with ongoing or failed builds.Tue, Feb 2, 12:34 AM
This revision was automatically updated to reflect the committed changes.