[PATCH] D21090: [PM] Port LCSSA to the new PM

[Davide Italiano via llvm-commits]

davide added inline comments.

================
Comment at: lib/Transforms/Utils/LCSSA.cpp:345
@@ +344,3 @@
+  // ported to the new PM and the legacy LoopSimplify pass has a call to
+  // mustPreserveAnalysisID(LCSSAID) in its runOnFunction method. So, for now,
+  // we are not preserving LoopSimplify.
----------------
davide wrote:
> mzolotukhin wrote:
> > eraman wrote:
> > > davide wrote:
> > > > This is a sore point. I think mustPreserveAnalysisID(LCSSAID) should become an assert in the new PM which verifies that the IR mutation didn't destroy LCSSA.
> > > > In other words, mustPreserveAnalysisID(LCSSAID) might need to go away.
> > > Could you elaborate on this a bit more? The current state is that loop transformation passes call mustPreserveAnalysisID(LCSSAID) to determine if LCSSA is available and ensures that the transformation preserves LCSSA by calling addPreserved. How will this look in the new PM?
> > > 
> > > Incidentally, LoopSimplify (and a few others) do not actually call addPreservedID(LCSSAID). Isn't this a bug?  
> > > 
> > > 
> > The fact that LoopSimplify doesn't do `addPreservedID(LCSSAID)` looks like a bug to me (which I'm trying to address in D21112 btw). What are the other passes that do the same?
> Sure.
> 1) My understanding is that LCSSA isn't really referential transparent/pure (in an haskell'y meaning of the word), and it's fairly easy to forge cases where it mutates the IR. Therefore, in the new (caching-based) PM, it's very hard to model the caching of LCSSA.
> What I think (and what I gathered from Chandler) is that in this case instead of caching we should just add a verifier pass which asserts LCSSA is maintained. 
> 
> 2) I think LoopSimplify should maintain LCSSA, and probably other passes in the LoopPM (which maybe don't today)
To be clear: when I say LCSSA is not pure I mean the LCSSA pass in LLVM, not the canonical form per-se.

Thanks!


http://reviews.llvm.org/D21090