[PATCH] D17864: [PGO] Promote indirect calls to conditional direct calls with value-profile

[Rong Xu via llvm-commits]

xur added inline comments.

================
Comment at: lib/Transforms/Instrumentation/IndirectCallPromotion.cpp:424
@@ +423,3 @@
+    int IX = PHI->getBasicBlockIndex(OrigBB);
+    if (IX == -1)
+      continue;
----------------
davidxl wrote:
> xur wrote:
> > davidxl wrote:
> > > Is this condition possible?
> > why not? The operand can come from a basic block other than OrigBB:
> > like bb1 (def v_1) --> bb2(invoke) --> bb
> >        bb2(def v_2) --> bb
> > bb can have a phi(v_1, v_2) in bb.
> > The incoming block for the phi with ix 0 is bb1, rather bb2, right?
> Sorry -- I misread it :)  WHat I meant is that the loop should have an assert that OrigBB is found in one of the iterations
It might not have a PHI that have OrigBB has the incoming BB, right?

================
Comment at: lib/Transforms/Instrumentation/IndirectCallPromotion.cpp:428
@@ +427,3 @@
+    PHI->addIncoming(V, IndirectCallBB);
+    PHI->setIncomingBlock(IX, DirectCallBB);
+  }
----------------
davidxl wrote:
> xur wrote:
> > davidxl wrote:
> > > What is the phi arg value for this new incoming block?
> > it's the same value as the one from OriginBB: in the two statements, the first only dup the value, and second one rest the incoming bb to DirectCallBB.
> IF the value was the value from the original indirect call, should the new value become the result of the direct call?
We have the special handling of the return value PHI. This only applies to the NormalDest. 

I assume UnwindBB won't use the return value. Let me know my assumption is incorrect.

This code handle the phi other than the return value PHI.


http://reviews.llvm.org/D17864