[PATCH] D69605: [Attributor] Make liveness "edge-based"

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert marked 4 inline comments as done.
jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2314
 
-    for (const Instruction *NRC : NoReturnCalls) {
+    for (const Instruction *NRC : ToBeExploredFrom) {
+      auto *CB = dyn_cast<CallBase>(NRC);
----------------
sstefan1 wrote:
> Maybe change `NRC` as well. It doesn't have real meaning after this rewrite.
Agreed, will do.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2481
   /// Collection of calls with noreturn attribute, assumed or knwon.
   SmallSetVector<const Instruction *, 4> NoReturnCalls;
 };
----------------
sstefan1 wrote:
> This doesn't seem to be used anymore.
yes.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2494
+  if (CB.isTerminator())
+    AliveSuccessors.push_back(&CB.getSuccessor(0)->front());
+  else
----------------
sstefan1 wrote:
> Just to make sure I understand this correctly. This is looking at `callbr` instruction and only looking at normal label? Shouldn't it look at other labels as well?
This is not called for `callbr` because of what you say. Only `CallInst` and `InvokeInst` end up here and the latter has some additional handling below.


================
Comment at: llvm/test/Transforms/FunctionAttrs/nonnull.ll:219
 ; FIXME: missing nonnull. It should be @f1(i32* nonnull readonly %arg)
-; ATTRIBUTOR:   %tmp = tail call nonnull i32* @f1(i32* readonly %arg)
+; ATTRIBUTOR:   %tmp = tail call nonnull i32* @f1(i32* %arg)
   %tmp = tail call i32* @f1(i32* %arg)
----------------
uenoku wrote:
> Why are these readonly removed?
they are not explicitly queries and now also not implicitly anymore. The information is not lost though. Maybe we want to add AAMemBehavior and others to *all* locations they are not seeded at the moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69605