[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;
 
----------------
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.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9460-9461
+
+      if (isa<InvokeInst>(CB))
+        return false;
+
----------------
At least add a TODO. To fix it proper you can look at the two successors blocks and the first non-debug instruction in them.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:9468-9470
+      for (auto It = Set.Unreachable.begin(); It != Set.Unreachable.end();) {
+        Function *Fn = *It;
+        It++;
----------------
Why no range loop?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106720