[PATCH] D57494: [CallSite removal] Remove CallSite uses from InstCombine.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Minor nits. I've been using `Call` instead of `CB` because when there is a nice word instead of initialism, I prefer it for readability. But that's up to you really.



================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:3984
 /// can eliminate the use of the cast.
-static bool isSafeToEliminateVarargsCast(const CallSite CS,
+static bool isSafeToEliminateVarargsCast(const CallBase &CB,
                                          const DataLayout &DL,
----------------
A const `CallSite` isn't actually the same as a `const CallBase &` -- that would be an `ImmutableCallSite`.

Still, if the const works here, great.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:4263
   // TODO: Do the transform if it only requires adding pointer casts.
-  if (CS.isMustTailCall())
+  if (isa<CallInst>(CB) && cast<CallInst>(CB).isMustTailCall())
     return false;
----------------
craig.topper wrote:
> Wasn't sure what to do here. isMustTailCall is inlined in CallSite, but it can't be inlined in CallBase.
You can just define it out-of-line. I don't think these *need* to be inlined, and in Instructions.cpp we have all the types available.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:4530
 
-  assert(Tramp &&
-         "transformCallThroughTrampoline called with incorrect CallSite.");
-
-  Function *NestF =cast<Function>(Tramp->getArgOperand(1)->stripPointerCasts());
+  Function *NestF =cast<Function>(Tramp.getArgOperand(1)->stripPointerCasts());
   FunctionType *NestFTy = cast<FunctionType>(NestF->getValueType());
----------------
Can use `clang-format` here. Also good candidate for `auto *NestF`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57494