[PATCH] D56143: [CallSite removal] Move the verifier to use `CallBase` instead of the `CallSite` wrapper.

Saleem Abdulrasool via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 31 22:40:25 PST 2018


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


================
Comment at: llvm/lib/IR/Verifier.cpp:492
 
-  void verifyCallSite(CallSite CS);
-  void verifySwiftErrorCallSite(CallSite CS, const Value *SwiftErrorVal);
+  void verifyCallBase(CallBase &Call);
+  void verifySwiftErrorCall(CallBase &Call, const Value *SwiftErrorVal);
----------------
I think it would be nicer to call it `verifyCall`.


================
Comment at: llvm/lib/IR/Verifier.cpp:2858
       // Allow sret here and check the wrapped function in verifyStatepoint.
-      if (CS.getCalledFunction() == nullptr ||
-          CS.getCalledFunction()->getIntrinsicID() !=
-            Intrinsic::experimental_gc_statepoint)
+      if (Call.getCalledFunction() == nullptr ||
+          Call.getCalledFunction()->getIntrinsicID() !=
----------------
I think that this might be easier to read:

```
if (!Call.getCalledFunction() || Call.getCalledFunction()->getIntrinsicID() != Intrinsic:::experimental_gc_statepoint)
```


================
Comment at: llvm/lib/IR/Verifier.cpp:2872
   // Verify that there's no metadata unless it's a direct call to an intrinsic.
-  if (CS.getCalledFunction() == nullptr ||
-      !CS.getCalledFunction()->getName().startswith("llvm.")) {
+  if (Call.getCalledFunction() == nullptr ||
+      !Call.getCalledFunction()->getName().startswith("llvm.")) {
----------------
Similar, use the negation than the `nullptr` check


================
Comment at: llvm/lib/IR/Verifier.cpp:2883
   // Verify that indirect calls don't return tokens.
-  if (CS.getCalledFunction() == nullptr)
+  if (Call.getCalledFunction() == nullptr)
     Assert(!FTy->getReturnType()->isTokenTy(),
----------------
Similar


================
Comment at: llvm/lib/IR/Verifier.cpp:4129
   // or are local to *this* function.
-  for (Value *V : CS.args())
+  for (Value *V : Call.args())
     if (auto *MD = dyn_cast<MetadataAsValue>(V))
----------------
Can't this be `const`?


================
Comment at: llvm/lib/IR/Verifier.cpp:4394
       // relocates of a call statepoint.
-      auto Token = CS.getArgOperand(0);
+      auto Token = Call.getArgOperand(0);
       Assert(isa<Instruction>(Token) && isStatepoint(cast<Instruction>(Token)),
----------------
Can't this be `const`?


================
Comment at: llvm/lib/IR/Verifier.cpp:4418
+    Assert(0 <= DerivedIndex && DerivedIndex < (int)StatepointCall.arg_size(),
+           "gc.relocate: statepoint derived index out of bounds", Call);
 
----------------
Mind switching to `static_cast<int>`?  `-Wold-style-cast` is .... annoying.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56143





More information about the llvm-commits mailing list