Preserve MemorySSA if it is available before running GVN.
DSE with MemorySSA will run closely after GVN. If GVN and 2 other
passes preserve MemorySSA, DSE can re-use MemorySSA used by LICM
when doing LTO.
Differential D86534
[GVN] Preserve MemorySSA if it is available. fhahn on Aug 25 2020, 5:56 AM. Authored by
Details Preserve MemorySSA if it is available before running GVN. DSE with MemorySSA will run closely after GVN. If GVN and 2 other
Diff Detail
Event TimelineComment Actions Not exactly the path I had in mind (working on newGVN was), but that's a longer avenue and I'm curious to see the compile-time impact of preserving MSSA :-)
Comment Actions Fix MemorySSA update for PRE loads. They are inserted just before the terminator, so we can just use createAccessInBB with BeforeTerminator position and the defining access of the original load. Ideally NewGVN would be ready, but unfortunately there are still a few bigger issues. I intend to pick this back up after wrapping up a few other things. Preserving MemorySSA is crucial to stay compile-time competitive with LTO, geomean ReleaseLTO -0.58%, ReleaseLTO (link only) -1.24% Comment Actions This comment has been deleted.
Comment Actions Changes look good. Comment Actions Thanks, I did a bootstrap build of LLVM with expensive MemorySSA verification and the test-suite with the set of patches to preserve MemSSA. The tests here and D86651 are from problems surfaced through that testing :) Comment Actions Hi! The following crashes for me with this patch with foo.ll being just define void @f() { entry: unreachable } I get opt: ../lib/IR/LegacyPassManager.cpp:657: void llvm::PMTopLevelManager::setLastUser(ArrayRef<llvm::Pass *>, llvm::Pass *): Assertion `AnalysisPass && "Expected analysis pass to exist."' failed. Comment Actions Thanks for the report. Looks like there is a problem with MemorySSAWrapperPass depending on AAResultsWrapperPass, which is not explicitly preserved by GVN. The problem can be solved by explicitly requiring AAResultsWrapperPass in EarlyCSE or by preserved AAResultsWrapper pass in GVN. I put up a patch for review/discussion for the best place to fix: D87137 |
I don't think this is right. Def can be a MemoryUse and that should not be a defining access.
I *think* it should take
MemoryAccess *Def2=isa<MemoryUse>(Def)? Def->getDefiningAccess():Def;
(with some proper var renaming)
This also shows MSSA should have an assert checking for this internally, I'll have to add that.
Please also add a MSSA verification at the end of the pass