[PATCH] D55641: [CallSite removal] Migrate all Alias Analysis APIs to use the newly minted `CallBase` class instead of the `CallSite` wrapper.

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd accepted this revision.
compnerd added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/include/llvm/Analysis/MemoryDependenceAnalysis.h:400
   /// that.
-  const NonLocalDepInfo &getNonLocalCallDependency(CallSite QueryCS);
+  const NonLocalDepInfo &getNonLocalCallDependency(CallBase *QueryCall);
 
----------------
Hmm, I realize that the `QueryCall` is used for an index via pointer, but, could we not take this by reference?  This shouldn't be null ever, so the reference would be nicer.


================
Comment at: llvm/include/llvm/Analysis/MemoryDependenceAnalysis.h:484
 private:
-  MemDepResult getCallSiteDependencyFrom(CallSite C, bool isReadOnlyCall,
-                                         BasicBlock::iterator ScanIt,
-                                         BasicBlock *BB);
+  MemDepResult getCallDependencyFrom(CallBase *Call, bool isReadOnlyCall,
+                                     BasicBlock::iterator ScanIt,
----------------
Similar.


================
Comment at: llvm/include/llvm/Analysis/MemoryLocation.h:234
   /// Return a location representing a particular argument of a call.
-  static MemoryLocation getForArgument(ImmutableCallSite CS, unsigned ArgIdx,
+  static MemoryLocation getForArgument(const CallBase *Call, unsigned ArgIdx,
                                        const TargetLibraryInfo *TLI);
----------------
Similar.


================
Comment at: llvm/lib/Analysis/AliasSetTracker.cpp:472
+
+      for (auto IdxArgPair : enumerate(Call->args())) {
+        int ArgIdx = IdxArgPair.index();
----------------
I wish that we could use something like `std::tie` to unpack in the loop declaration as in C++17.


================
Comment at: llvm/lib/Analysis/AliasSetTracker.cpp:474
+        int ArgIdx = IdxArgPair.index();
+        const Value *Arg = IdxArgPair.value();
+        if (!Arg->getType()->isPointerTy())
----------------
I think that using `std::tie` to unpack is nicer.


================
Comment at: llvm/lib/Analysis/CaptureTracking.cpp:275
+        int Idx = IdxOpPair.index();
+        Value *A = IdxOpPair.value();
+        if (A == V && !Call->doesNotCapture(Idx))
----------------
I think that `std::tie` is nicer.


================
Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:160
       // ignore calls that only access local memory.
-      for (CallSite::arg_iterator CI = CS.arg_begin(), CE = CS.arg_end();
+      for (CallSite::arg_iterator CI = Call->arg_begin(), CE = Call->arg_end();
            CI != CE; ++CI) {
----------------
Why not convert this to a range based for loop:?


================
Comment at: llvm/lib/Transforms/Scalar/GVN.cpp:440
     const MemoryDependenceResults::NonLocalDepInfo &deps =
-      MD->getNonLocalCallDependency(CallSite(C));
+        MD->getNonLocalCallDependency(C);
     // FIXME: Move the checking logic to MemDep!
----------------
Might be nice to make that down-cast to a `CallBase` explicit.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55641