This is an archive of the discontinued LLVM Phabricator instance.

Refactor the build{Module|Function}SimplificationPipeline to expose optimization phase.
ClosedPublic

Authored by danielcdh on Jul 29 2017, 6:20 PM.

Event Timeline

danielcdh created this revision.Jul 29 2017, 6:20 PM
chandlerc edited edge metadata.Jul 29 2017, 7:35 PM

I definitely lik ethis direction. Minor comments about wording / phrasing of things below.

include/llvm/Passes/PassBuilder.h
74–77

I don't think we want this to be so open ended. Until there is some *other* necessary predicate, we should scope this very narrowly so it isn't tempting to use for other things without careful consideration.

I'd call this ThinLTOPhase. Also, I'd make it an enum class.

78–79

Based on the above, this would likely become None with the comment that it indicates no ThinLTO behavior needed.

81–90

And these can just be PreLink and PostLink as they'll have a qualifier when used: ThinLTOPhase::PreLink.

I think the comments should also be minimal. The details of how PGO being listed here seem risky to just fall out of sync with the reality. I'd either say nothing (because PreLink seems pretty clear) or that they correspond to the pre-link phase of ThinLTO.

danielcdh updated this revision to Diff 108813.Jul 29 2017, 7:55 PM
danielcdh marked 3 inline comments as done.

update

chandlerc accepted this revision.Jul 29 2017, 8:33 PM

LGTM with a comment wording tweak below. Thanks for he refactoring!

include/llvm/Passes/PassBuilder.h
229

"the current optimization phase" -> "the current ThinLTO phase", here and below where the same phrasing is used.

This revision is now accepted and ready to land.Jul 29 2017, 8:33 PM
danielcdh updated this revision to Diff 108814.Jul 29 2017, 8:39 PM
danielcdh marked an inline comment as done.

update. Thanks for the review!

danielcdh closed this revision.Jul 29 2017, 9:56 PM