[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:9435
+      if (!Reachability.isAssumedReachable(A, CBInst, Inst))
+        return true;
 
----------------
kuter wrote:
> jdoerfert wrote:
> > Is this the right way? I would have thought we ignore CB if CBInst is not reachable from Inst, not the other way around. We need a test I suppose.
> We are doing an early return if the CB is not assumed reachable. If not we add it into the `Result` Vector.  We later use the result of this function for  `QueryResolver::update` or `QueryResolver::isReachable`.
> 
> So the `return true` here doesn't mean anything.
I was not clear. My concern is that we check if `CBInst` may reach `Inst` while we want to know if `Inst` may reach `CBInst`, no?
So `if (!Reachability.isAssumedReachable(A, Inst, CBInst))` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106720