[PATCH] D22182: Refactor indirect call promotion profitability analysis (NFC)

David Li via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 8 20:55:29 PDT 2016


davidxl added inline comments.

================
Comment at: include/llvm/Analysis/IndirectCallPromotionAnalysis.h:60
@@ +59,3 @@
+  ArrayRef<InstrProfValueData>
+  getPromotionCandidatesForInstruction(const Instruction *I, uint32_t &NumVals,
+                                       uint64_t &TotalCount,
----------------
This analysis does not check legality, why is that?

================
Comment at: lib/Transforms/Instrumentation/IndirectCallPromotion.cpp:149
@@ -190,3 +148,3 @@
       Instruction *Inst, const ArrayRef<InstrProfValueData> &ValueDataRef,
-      uint64_t TotalCount);
+      uint64_t TotalCount, uint32_t NumCandidates);
 
----------------
Document the parameters including the new one.

================
Comment at: lib/Transforms/Instrumentation/IndirectCallPromotion.cpp:262
@@ -312,11 +261,3 @@
 
-    if (ICPInvokeOnly && dyn_cast<CallInst>(Inst)) {
-      DEBUG(dbgs() << " Not promote: User options.\n");
-      break;
-    }
-    if (ICPCallOnly && dyn_cast<InvokeInst>(Inst)) {
-      DEBUG(dbgs() << " Not promote: User option.\n");
-      break;
-    }
     if (ICPCutOff != 0 && NumOfPGOICallPromotion >= ICPCutOff) {
       DEBUG(dbgs() << " Not promote: Cutoff reached.\n");
----------------
I think there is a bug in the original code. This check should be moved after legality check below. The update of NumOfPGOICallPromotion should also be done here instead of in tryToPromote.

But this can be fixed in a different patch.


http://reviews.llvm.org/D22182





More information about the llvm-commits mailing list