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

[Rong Xu via llvm-commits]

xur added inline comments.

================
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");
----------------
davidxl wrote:
> 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.
Which check are you referring to?
These early exits seem good to me: for the *Only check, the icall type only depends on the instruction. it has nothing to do with legality.
For the cutoff check, only it reaches, we can safely exit without doing the other checks. why you think this is a bug?

================
Comment at: lib/Transforms/Instrumentation/IndirectCallPromotion.cpp:595
@@ -661,1 +594,3 @@
+    annotateValueSite(*M, *I, ICallProfDataRef.slice(NumPromoted), TotalCount,
+                      IPVK_IndirectCallTarget, NumPromoted);
   }
----------------
is the last argument correct? I think it should be NumCandidates.


http://reviews.llvm.org/D22182