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

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Analysis/AliasAnalysis.cpp:886
   AU.setPreservesAll();
-  AU.addRequired<BasicAAWrapperPass>();
-  AU.addRequired<TargetLibraryInfoWrapperPass>();
+  AU.addRequiredTransitive<BasicAAWrapperPass>();
+  AU.addRequiredTransitive<TargetLibraryInfoWrapperPass>();
----------------
I think this makes sense because a BasicAAResult object can be added to the AAResults object. But what about all the other kinds of AA object that can (optionally) be added? E.g. if we add a SCEVAAResult, doesn't that mean that this pass needs to "transitively require if available" SCEVAAWrapperPass?


================
Comment at: llvm/lib/Analysis/AliasAnalysis.cpp:887
+  AU.addRequiredTransitive<BasicAAWrapperPass>();
+  AU.addRequiredTransitive<TargetLibraryInfoWrapperPass>();
 
----------------
This makes sense to me, given that the AAResults object has a reference to TLI.


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1878-1880
+  AU.addRequiredTransitive<AssumptionCacheTracker>();
+  AU.addRequiredTransitive<DominatorTreeWrapperPass>();
+  AU.addRequiredTransitive<TargetLibraryInfoWrapperPass>();
----------------
This makes sense to me, given that the BasicAAResult object has references or pointers to TLI and AC and DT. I wonder how we have got away with it for so long?


================
Comment at: llvm/lib/Target/Hexagon/HexagonLoopIdiomRecognition.cpp:168
     AU.addRequired<AAResultsWrapperPass>();
-    AU.addPreserved<AAResultsWrapperPass>();
     AU.addRequired<ScalarEvolutionWrapperPass>();
----------------
Did you try the alternative fix of preserving more stuff instead of less? I.e. preseve everything that is transitively required by AAResultsWrapperPass? Is that hard to do?


================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:550
     AU.addPreserved<GlobalsAAWrapperPass>();
-    AU.addPreserved<AAResultsWrapperPass>();
   }
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94138