Page MenuHomePhabricator

[NewPM] Add pipeline EP callback after initial frontend cleanup
ClosedPublic

Authored by aeubanks on Nov 19 2020, 9:40 AM.

Details

Summary

This matches the legacy PM's EP_ModuleOptimizerEarly. Some backends use
this extension point and adding the pass somewhere else like
PipelineStartEPCallback doesn't work.

Diff Detail

Event Timeline

aeubanks created this revision.Nov 19 2020, 9:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 19 2020, 9:40 AM
aeubanks requested review of this revision.Nov 19 2020, 9:40 AM

FWIW, could we keep naming it ModuleOptimizerEarlyEPCallback? That sounds pretty intuitive to me and reminds users of the old name. Is the renaming for using the prefix Pipeline?

FWIW, could we keep naming it ModuleOptimizerEarlyEPCallback? That sounds pretty intuitive to me and reminds users of the old name. Is the renaming for using the prefix Pipeline?

ModuleOptimizerEarly is sort of a relic of the legacy PM. The legacy PassManagerBuilder has a populateFunctionPassManager() for cleaning up input IR and populateModulePassManager() for the real pipeline, but there's no equivalent of populateFunctionPassManager() in the NPM. So the ModuleOptimizer part of ModuleOptimizerEarlyEPCallback wouldn't make sense IMO.

I will agree that the name I chose could be better named, but still unsure of anything better.

I don't think "ModuleOptimizer" in the name makes sense, the association can come from the "Early" part. Either keep the naming as is, or "EarlyAfterStart" if you want to define order against the other callbacks.
ychen@ so you have a better naming suggestion?

ychen added a comment.Nov 23 2020, 4:34 PM

I don't think "ModuleOptimizer" in the name makes sense, the association can come from the "Early" part. Either keep the naming as is, or "EarlyAfterStart" if you want to define order against the other callbacks.
ychen@ so you have a better naming suggestion?

Having a second look, yeah, I agree that "ModuleOptimizer" in the name does not make sense. Early is also vague about where it is called. How about just ModuleSimplificationEPCallback?

EarlySimplificationEPCallback?

ychen added a comment.Nov 24 2020, 6:17 PM

EarlySimplificationEPCallback?

sounds good to me.

aeubanks updated this revision to Diff 307495.Nov 24 2020, 6:48 PM

rename to EarlySimplification

ychen accepted this revision.Nov 24 2020, 7:09 PM

LGTM. Thanks!

This revision is now accepted and ready to land.Nov 24 2020, 7:09 PM