[PATCH] D78246: [SampleProfile] Use CallBase in function arguments and data structures to reduce the number of explicit casts. NFCI

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 16 12:50:18 PDT 2020


dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good (the few couple of suggestions of collapsing isa+cast /might/ be wrong if they're really intentionally trying to restrict to just just Call+Invoke, not CallBrInst? in which case collapsing them would be wrong, because it'd include CallBrInst - but I don't think that's the case)



================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:722-725
   if ((isa<CallInst>(Inst) || isa<InvokeInst>(Inst)) &&
-      !cast<CallBase>(Inst).isIndirectCall() && findCalleeFunctionSamples(Inst))
+      !cast<CallBase>(Inst).isIndirectCall() &&
+      findCalleeFunctionSamples(cast<CallBase>(Inst)))
     return 0;
----------------
Could maybe restructure this to:
```
if (auto *CB = dyn_cast<CallBase>(&Inst))
  if (CB->isIndirectCall() && findCalleeFunctionSamples(CB))
    return 0;
```


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1054-1055
             // If profile mismatches, we should not attempt to inline DI.
             if ((isa<CallInst>(DI) || isa<InvokeInst>(DI)) &&
-                inlineCallInstruction(DI)) {
+                inlineCallInstruction(*cast<CallBase>(DI))) {
               localNotInlinedCallSites.erase(I);
----------------
again, might be nice to "if (dyn_cast) + if(inlineCallInstruction)"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78246





More information about the llvm-commits mailing list