[PATCH] D40751: [ICP] Expose unconditional call promotion interface

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: include/llvm/Transforms/Utils/CallPromotionUtils.h:47
+/// call were direct.
+void demoteCall(CallSite CS, Value *CalledValue, ArrayRef<CastInst *> Casts);
+
----------------
Can you remove this method in this patch? It is not used anywhere


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:829
             uint64_t C = FS->getEntrySamples();
-            Instruction *DI =
-                pgo::promoteIndirectCall(I, R->getValue(), C, Sum, false, ORE);
+            Instruction *DI = I;
+            I = pgo::promoteIndirectCall(I, R->getValue(), C, Sum, false, ORE);
----------------
what is this change for?


================
Comment at: lib/Transforms/Instrumentation/IndirectCallPromotion.cpp:315
     Weights.push_back(Count);
-    MDBuilder MDB(NewInst->getContext());
-    NewInst->setMetadata(LLVMContext::MD_prof, MDB.createBranchWeights(Weights));
+    MDBuilder MDB(Inst->getContext());
+    Inst->setMetadata(LLVMContext::MD_prof, MDB.createBranchWeights(Weights));
----------------
What is this change for?


================
Comment at: lib/Transforms/Utils/CallPromotionUtils.cpp:36
       continue;
-    Value *V = Phi->getIncomingValue(Idx);
-    if (dyn_cast<Instruction>(V) == Invoke) {
-      Phi->setIncomingBlock(Idx, ElseBlock);
-      Phi->addIncoming(NewInst, OrigBlock);
-      continue;
-    }
-    Phi->addIncoming(V, ElseBlock);
+    Phi->setIncomingBlock(Idx, MergeBlock);
   }
----------------
Can you explain this change (and other related interface changes)  perhaps with a tiny example -- there are lots of words in the description but not as helpful as an example.


================
Comment at: lib/Transforms/Utils/CallPromotionUtils.cpp:279
+
+void llvm::demoteCall(CallSite CS, Value *CalledValue,
+                      ArrayRef<CastInst *> Casts) {
----------------
Remove this one from this patch.


https://reviews.llvm.org/D40751