[PATCH] D27288: [GVN] When merging blocks update LoopInfo if it's available

[Adam Nemet via Phabricator via llvm-commits]

anemet added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Scalar/GVN.cpp:2697
 
+    auto *LIWP = getAnalysisIfAvailable<LoopInfoWrapperPass>();
+
----------------
efriedma wrote:
> anemet wrote:
> > efriedma wrote:
> > > This doesn't make sense... getAnalysisUsage says that GVN doesn't preserve LoopInfo, so it shouldn't be required to preserve it.  How exactly is this causing a problem?
> > This is not about preserving for subsequent passes but preserving while running GVN.  The reason is that BasicAA uses LI if it's available and it is using it *lazily*, i.e. holding onto the analysis pass rather than querying its result right away.
> > 
> > What's changed is that LI is kept alive during GVN now because a new analysis pass OptimizationRemarkEmitter indirectly uses LI.  As a consequence, BasicAA will now use LI, so we need to keep it up-to-date during GVN so that BasicAA uses consistent state.
> > 
> > I believe that this is a known problem with analysis passes holding on to other analysis passes and using them lazily.  The "lazy" dependencies will have to be kept consistent downstream.
> Oh, I see, GVN requires OptimizationRemarkEmitter, OptimizationRemarkEmitter requires LI, therefore GVN must preserve LI.  That's fine, I guess, although it's pretty subtle.
> 
> Could you add an explicit call to `addPreserved<LoopInfoWrapperPass>` for documentation purposes?
Yes it is subtle, and I should have added a comment about this.  Will do in a follow-up (this is already committed).

> Could you add an explicit call to addPreserved<LoopInfoWrapperPass> for documentation purposes?

Makes sense, will do that in a follow-up too.


Repository:
  rL LLVM

https://reviews.llvm.org/D27288