[PATCH] D118673: [Attributor] Introduce the `AA::isPotentiallyReachable` helper APIs

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:471
+        return true;
+      continue;
+    }
----------------
kuter wrote:
> I don't think this would work for recursive functions  when doing backwards reachability.
> the next instruction might contain a call that will reach `ToI`
I don't understand what the problem is.
If `CurFromI->getNextNode()` can reach `ToI`, the query `isAssumedReachable(A, *CurFromI, *ToI);` will be true.
Other calls that end up in the `ToFn`, e.g., going through recursion, are handled by the continue here instead of a return false.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:482
+                        << *CurFromI << " is not checked backwards, abort\n");
+      return true;
+    }
----------------
kuter wrote:
> We haven't called `FnReachabilityAA.instructionCanReach` yet. This function won't do interprocedural reachability if there is no GoBackwardsCB  ?
> 
Yes, as the comment states, if there is no backwards CB we give up. Nothing right now will otherwise cause this to not return "may reach" anyway. We eventually hit an external function and the callers are unknown and we give up. Once we have logic to determine an internal function can only be reached from specific places, etc. we can do more, unclear if that will be here though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118673