Page MenuHomePhabricator

Refactor the code to pass down ACT to SampleProfileLoader correctly.
ClosedPublic

Authored by danielcdh on Sep 12 2017, 2:24 PM.

Details

Summary

This change passes down ACT to SampleProfileLoader for the new PM. Also remove the default value for SampleProfileLoader class as it is not used.

Event Timeline

danielcdh created this revision.Sep 12 2017, 2:24 PM
eraman added inline comments.Sep 12 2017, 2:43 PM
lib/Transforms/IPO/SampleProfile.cpp
150

Name = SampleProfileFile ? Looking below, this is a separate refactoring. Perhaps separate the two out? Otherwise, at least make the desccription clear that there are two different refactoring.

753

Any reason why you want a named temporary?

1485

Since ACT is an immutable pass that doesn't require the module, you can move the initialization of ACT to the constructor of the legacy pass.

1512–1518

you don't need the std::function here. Just use auto.

danielcdh edited the summary of this revision. (Show Details)Sep 12 2017, 2:51 PM
danielcdh updated this revision to Diff 114916.Sep 12 2017, 2:51 PM
danielcdh marked 2 inline comments as done.

update

eraman accepted this revision.Sep 12 2017, 2:56 PM

LGTM

lib/Transforms/IPO/SampleProfile.cpp
1485

(Dehao explained offline why this won't work)

This revision is now accepted and ready to land.Sep 12 2017, 2:56 PM
danielcdh added inline comments.Sep 12 2017, 2:57 PM
lib/Transforms/IPO/SampleProfile.cpp
753

revert, this was in preparation for the follow-up patch.

1485

Discussed offline, this need to be here (after calling getAnalysisUsage)

danielcdh closed this revision.Sep 12 2017, 2:57 PM