[PATCH] D84665: [Attributor][WIP] Implement AAReachability

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:895
+      KnownReachableQueries;
+
   /// Give the Attributor access to the members so
----------------
Why don't these life in the AAReachabilityFunction?


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:2359
+  /// See AbstractAttribute::initialize(...).
+  void initialize(Attributor &A) override {}
+
----------------
Remove this and the initialize in the Impl class.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:2365
+    // to be unreachable BB pair. Return CHANGED when at least one query turn out to
+    // be reachable BB pair.
+    ChangeStatus Changed = ChangeStatus::UNCHANGED;
----------------
typo above.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:2368
+    const auto &FnLiveness = A.getAAFor<AAIsDead>(*this, getIRPosition());
+    using QueryTy = std::pair<const BasicBlock *, const BasicBlock *>;
+    DenseSet<QueryTy> &AssumedUnreachableQueries =
----------------
Define this as EdgeTy or similar in the class once you moved the query vectors there.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:2407
+        Worklist.push_back(*SuccIt);
+        VisitedBBs.insert(*SuccIt);
+      }
----------------
Don't use count above but the second element of the return value of insert.


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

https://reviews.llvm.org/D84665