[PATCH] D94138: Require chained analyses in BasicAA and AAResults to be transitive

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:550
     AU.addPreserved<GlobalsAAWrapperPass>();
-    AU.addPreserved<AAResultsWrapperPass>();
   }
----------------
nikic wrote:
> bjope wrote:
> > foad wrote:
> > > Ditto.
> > I did try some different solutions here, but never really figured out exactly what was needed to avoid the assert in setLastUser (also knowing that those transitive analyses actually are preserved).
> > The solution of not preserving AAResults was inspired by the change above for HexagonLoopIdiomRecognition, but it also matches what it looks like in GVN and NewGVN etc.
> As this already preserves DT, possibly this only needs an explicit `AU.addPreserved<BasicAAWrapperPass>`?
Yes, adding `AU.addPreserved<BasicAAWrapperPass>` seem to work as well. However, my intention is to land this as is. The NPM version does not preserve AAResults nor BasicAA so it makes the implementation a bit more equal between the two pass manager implementations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94138/new/

https://reviews.llvm.org/D94138