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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:8707
+      Instruction *Inst =
+          CallSite.getInstruction()->getNextNonDebugInstruction();
+      const AAFunctionReachability &AA = A.getAAFor<AAFunctionReachability>(
----------------
I think this is a problem if you have invokes.
Dies the instCanReach include the instruction or not? If it's the latter we can just pass the call site.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:8723
+    bool AllCallSitesKnown = false;
+    A.checkForAllCallSites(CheckCallSite, *this, true, AllCallSitesKnown);
+
----------------
don't ignore the return value


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:8731
+      }
+    }
+
----------------
no braces.


================
Comment at: llvm/unittests/Transforms/IPO/AttributorTest.cpp:204
+  // Because func10 calls the func4 after the call to func9 it is reachable.
+  ASSERT_TRUE(F9AA.instCanReach(A, F9FirstInst, F4));
 }
----------------
can we have a negative instcanreach test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106720