This is an archive of the discontinued LLVM Phabricator instance.

Set option default for enabling memory ssa for new pass manager loop sink pass to true.
ClosedPublic

Authored by jamieschmeiser on Dec 2 2020, 7:34 AM.

Details

Summary

Set the default for the option enabling memory ssa use in the loop sink
pass to true for the new pass manager.

Diff Detail

Event Timeline

jamieschmeiser created this revision.Dec 2 2020, 7:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2020, 7:34 AM
jamieschmeiser requested review of this revision.Dec 2 2020, 7:34 AM
nikic added a comment.Dec 2 2020, 9:26 AM

Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=54eab293f523956bdc4b1a98b6cf5abc0bd1ef3f&to=b023c8adc9eb545ec5b8aa1a8d6f77a46d622006&stat=instructions

The problem here is that LoopSink is only relevant for PGO, but MemorySSA gets computed unconditionally in the legacy PM. This problem does not affect the new pass manager, where it's possible to compute MemorySSA only if useful. Supporting this properly in LegacyPM would require something like LazyMemorySSA.

I think in this instance it might be best to do this change only for NewPM. It's best to keep both pass managers in sync, but here there is a technical reason for different treatment.

fhahn added a subscriber: fhahn.Dec 2 2020, 9:46 AM

Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=54eab293f523956bdc4b1a98b6cf5abc0bd1ef3f&to=b023c8adc9eb545ec5b8aa1a8d6f77a46d622006&stat=instructions

The problem here is that LoopSink is only relevant for PGO, but MemorySSA gets computed unconditionally in the legacy PM. This problem does not affect the new pass manager, where it's possible to compute MemorySSA only if useful. Supporting this properly in LegacyPM would require something like LazyMemorySSA.

I think in this instance it might be best to do this change only for NewPM. It's best to keep both pass managers in sync, but here there is a technical reason for different treatment.

Perhaps it would also be possible to directly construct MemorySSA in the pass, if PGO is enabled with the legacy PM?

nikic added a comment.Dec 2 2020, 9:51 AM

Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=54eab293f523956bdc4b1a98b6cf5abc0bd1ef3f&to=b023c8adc9eb545ec5b8aa1a8d6f77a46d622006&stat=instructions

The problem here is that LoopSink is only relevant for PGO, but MemorySSA gets computed unconditionally in the legacy PM. This problem does not affect the new pass manager, where it's possible to compute MemorySSA only if useful. Supporting this properly in LegacyPM would require something like LazyMemorySSA.

I think in this instance it might be best to do this change only for NewPM. It's best to keep both pass managers in sync, but here there is a technical reason for different treatment.

Perhaps it would also be possible to directly construct MemorySSA in the pass, if PGO is enabled with the legacy PM?

I think that would require constructing MemorySSA anew for each loop. I'm not really familiar with pass manager limitations though, maybe this is not the case.

jamieschmeiser retitled this revision from Set option default for enabling memory ssa for loop sink pass to true. to Set option default for enabling memory ssa for new pass manager loop sink pass to true..
jamieschmeiser edited the summary of this revision. (Show Details)

Remove changes pertaining to legacy pass manager.

jamieschmeiser edited the summary of this revision. (Show Details)Dec 2 2020, 12:09 PM
jamieschmeiser edited the summary of this revision. (Show Details)
asbirlea accepted this revision.Jan 6 2021, 6:30 PM

I believe for the NPM this should work fine.
@nikic @fhahn Any objections?

This revision is now accepted and ready to land.Jan 6 2021, 6:30 PM