Page MenuHomePhabricator

[LICM] Don't cache AliasSetTrackers when run under legacy PM
ClosedPublic

Authored by DaniilSuchkov on Jan 21 2020, 12:08 AM.

Details

Summary

This is the first step towards complete removal of AST caching from LICM.
Attempts to keep LICM's AST cache up to date across passes can lead to miscompiles like this one: https://bugs.llvm.org/show_bug.cgi?id=44320
Given LICM has already switched to building ASTs using MemorySSA, we don't have compile-time reasons to keep AST caching any more.
This switch should help us to surface any possible issues that may arise along this way (maybe there are still some compile-time critical uses of LICM out there without access to LICM?), also it turns subsequent removal of AST caching into NFC.

Diff Detail

Event Timeline

DaniilSuchkov created this revision.Jan 21 2020, 12:08 AM
DaniilSuchkov edited the summary of this revision. (Show Details)Jan 21 2020, 3:01 AM
asbirlea accepted this revision.Jan 21 2020, 9:44 AM

Thanks you for taking this on!

Could you add a few more details to the motivation and current state of the codebase, something along the lines:

LICM has already switched to using MemorySSA to do sinking and hoisting and only builds an AliasSetTracker on demand for the promoteToScalars step, without caching it from one LICM instance to the next. Given this, we don't have compile-time reasons to keep AST caching any more.
The only scenario where the caching would be used currently is when using the LegacyPassManager and setting -enable-mssa-loop-dependency=false.
This revision is now accepted and ready to land.Jan 21 2020, 9:44 AM
This revision was automatically updated to reflect the committed changes.