[PATCH] D106720: [Attributor] AAFunctionReachability, Instruction reachability.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9659-9664
+    bool NoUnknownCall = true;
+    NoUnknownCall &=
+      A.checkForAllCallSites(CheckCallSite, *this, true, NoUnknownCall);
+
+    // If we don't know all callsites we have to assume that we can reach fn.
+    if (!NoUnknownCall) {
----------------



================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9740
+    NonConstThis->checkReachableBackwards(A, InstBackwardsQSet);
+    return InstBackwardsQSet.Reachable.count(&Fn);
+  }
----------------
Reachable is not updated if `CanReachUnknownCallee` is set true. We also never check for a cached value here.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9751
 
     for (auto CBPair : CBQueries) {
       const AACallEdges &AAEdges = A.getAAFor<AACallEdges>(
----------------
One of these caused a copy and only the copy QuerySet got updated causing it never to find a fixpoint.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9768
+    // Check for local callbases first.
+    for (auto InstPair : InstQueries) {
+      SmallVector<const AACallEdges *> CallEdges;
----------------
One of these caused a copy and only the copy QuerySet got updated causing it never to find a fixpoint.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9778
+
+    for (auto QueryPair : InstQueriesBackwards)
+      Change |= checkReachableBackwards(A, QueryPair.second);
----------------
One of these caused a copy and only the copy QuerySet got updated causing it never to find a fixpoint.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106720