[PATCH] D38477: Annotate VP prof on indirect call if it is ICPed in the profiled binary.

[Dehao Chen via Phabricator via llvm-commits]

danielcdh added inline comments.


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:514
 
   // If a call/invoke instruction is inlined in profile, but not inlined here,
   // it means that the inlined callsite has no sample, thus the call
----------------
tejohnson wrote:
> How do we know it was inlined in the profile from the below checks?
Updated the comment to make it clear.


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:518
   if ((isa<CallInst>(Inst) || isa<InvokeInst>(Inst)) &&
+      !ImmutableCallSite(&Inst).isIndirectCall() &&
       findCalleeFunctionSamples(Inst))
----------------
tejohnson wrote:
> Comment needs update. It isn't clear to me why indirect calls are handled differently. I understand that you want to annotate the fallback indirect call with any VP metadata it collected. But couldn't the promoted/inlined indirect call in the profiled binary have the same situation (inlined callsite had no sample)? I think this goes back to my first question above, about how we know it was inlined in the profile from here. 
Comment updated to make it clear.

Basically this only applies to direct call. For indirect call, even if the call is promoted and inlined, we will still get profile for the indirect call, so should not blindly set it as 0.


https://reviews.llvm.org/D38477