[PATCH] D27288: [GVN] When merging blocks update LoopInfo if it's available
Adam Nemet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 1 13:25:21 PST 2016
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
More information about the llvm-commits
mailing list