[PATCH] D77930: [llvm][NFC] Refactor uses of CallSite to CallBase - call promotion

Mircea Trofin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Apr 11 19:13:00 PDT 2020


mtrofin marked 2 inline comments as done.
mtrofin added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUFixFunctionBitcasts.cpp:39
+    if (Callee &&
+        isLegalToPromote(*cast<CallBase>(CS.getInstruction()), Callee)) {
+      promoteCall(*cast<CallBase>(CS.getInstruction()), Callee);
----------------
dblaikie wrote:
> Hmm, might be worth adding an implicit conversion from CallSite to CallBase* to make code like this easier? (alternatively/at least CallSite::getInstruction() should return CallBase*, I think?)
I'd keep it ugly and hard to use, since we want to deprecate CallSite, and encourage more refactoring.

(I'm hoping to actually complete it, I think it's very tractable)


================
Comment at: llvm/unittests/Transforms/Utils/CallPromotionUtilsTest.cpp:95-98
+  CallBase &CB = *CI;
+  ASSERT_FALSE(CB.getCalledFunction());
+  bool IsPromoted = tryPromoteCall(CB);
   EXPECT_FALSE(IsPromoted);
----------------
dblaikie wrote:
> Can probably drop the intermediate CB variable now (& just write CI->getCalledFunction() and tryPromoteCall(*CI))
> 
> (similarly below)
Done.

Also, seems like this file would benefit from ASSERT_TRUE(pointer) -> ASSERT_NE(pointer, nullptr) (same for EXPECT_) - or is this not particularly followed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77930





More information about the llvm-commits mailing list