Annotate VP prof on indirect call if it is ICPed in the profiled binary.

[danielcdh]

Summary

In SamplePGO, when an indirect call is promoted in the profiled binary, before profile annotation, it will be promoted and inlined. For the original indirect call, the current implementation will not mark VP profile on it. This is an issue when profile becomes stale. This patch annotates VP prof on indirect calls during annotation.

[tejohnson]

Summary has a couple of typos

lib/Transforms/IPO/SampleProfile.cpp

514–515

How do we know it was inlined in the profile from the below checks?

519

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.

[danielcdh]

update comment

[danielcdh]

lib/Transforms/IPO/SampleProfile.cpp

514–515

Updated the comment to make it clear.

519

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.

[tejohnson]

LGTM
(please just fix the 2 typos in the patch summary before submitting)

include/llvm/IR/CallSite.h

public:
   /// Return the function being called if this is a direct call, otherwise
   /// return null (if it's an indirect call).
   FunTy *getCalledFunction() const {
     return dyn_cast<FunTy>(getCalledValue());
   }
 
   /// Return true if the callsite is an indirect call.
   bool isIndirectCall() const {
-    Value *V = getCalledValue();
+    const Value *V = getCalledValue();
     if (!V)
       return false;
     if (isa<FunTy>(V) || isa<Constant>(V))
       return false;
-    if (CallInst *CI = dyn_cast<CallInst>(getInstruction())) {
+    if (const CallInst *CI = dyn_cast<CallInst>(getInstruction())) {
       if (CI->isInlineAsm())
         return false;
     }
     return true;
   }
 
   /// Set the callee to the specified value.
   void setCalledFunction(Value *V) {

lib/Transforms/IPO/SampleProfile.cpp

if (!FS)
     return std::error_code();
 
   // Ignore all intrinsics and branch instructions.
   // Branch instruction usually contains debug info from sources outside of
   // the residing basic block, thus we ignore them during annotation.
   if (isa<BranchInst>(Inst) || isa<IntrinsicInst>(Inst))
     return std::error_code();
 
-  // If a call/invoke instruction is inlined in profile, but not inlined here,
+  // If a direct call/invoke instruction is inlined in profile
+  // (findCalleeFunctionSamples returns non-empty result), but not inlined here,
   // it means that the inlined callsite has no sample, thus the call
   // instruction should have 0 count.
   if ((isa<CallInst>(Inst) || isa<InvokeInst>(Inst)) &&
+      !ImmutableCallSite(&Inst).isIndirectCall() &&
       findCalleeFunctionSamples(Inst))
     return 0;
 
   const DILocation *DIL = DLoc;
   uint32_t LineOffset = getOffset(DIL);
   uint32_t Discriminator = DIL->getBaseDiscriminator();
   ErrorOr<uint64_t> R = FS->findSamplesAt(LineOffset, Discriminator);
   if (R) {

test/Transforms/SampleProfile/Inputs/indirect-call.prof

 test:63067:0
  1: 3345 _Z3barv:1398 _Z3foov:2059
 test_inline:3000:0
+ 1: 1000 foo_inline3:1000
  1: foo_inline1:3000
   11: 3000
  1: foo_inline2:4000
   19: 4000
 test_noinline:3000:0
  1: foo_noinline:3000
   20: 3000
 test_direct:3000:0

test/Transforms/SampleProfile/indirect-call.ll

 ; CHECK: icmp {{.*}} @foo_inline2
 ; CHECK: if.true.direct_targ:
 ; CHECK-NOT: call
 ; CHECK: if.false.orig_indirect:
 ; CHECK: icmp {{.*}} @foo_inline1
 ; CHECK: if.true.direct_targ1:
 ; CHECK-NOT: call
 ; CHECK: if.false.orig_indirect2:
-; CHECK: call
+; CHECK: call {{.*}} !prof ![[VP:[0-9]+]]
   call i64* %3(i32* %x), !dbg !7
   ret void
 }
 
 ; CHECK-LABEL: @test_inline_strip
 ; If the indirect call is promoted and inlined in profile, and the callee name
 ; is stripped we should promote and inline it.
 define void @test_inline_strip(i64* (i32*)*, i32* %x) !dbg !8 {

 
 !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1)
 !1 = !DIFile(filename: "test.cc", directory: "/")
 !2 = !{i32 2, !"Debug Info Version", i32 3}
 !3 = distinct !DISubprogram(name: "test", scope: !1, file: !1, line: 3, unit: !0)
 !4 = !DILocation(line: 4, scope: !3)
 !5 = !DILocation(line: 6, scope: !3)
 ; CHECK: ![[PROF]] = !{!"VP", i32 0, i64 3457, i64 9191153033785521275, i64 2059, i64 -1069303473483922844, i64 1398}
+; CHECK: ![[VP]] = !{!"VP", i32 0, i64 1000, i64 -6391416044382067764, i64 1000}
 !6 = distinct !DISubprogram(name: "test_inline", scope: !1, file: !1, line: 6, unit: !0)
 !7 = !DILocation(line: 7, scope: !6)
 !8 = distinct !DISubprogram(name: "test_inline_strip", scope: !1, file: !1, line: 8, unit: !0)
 !9 = !DILocation(line: 9, scope: !8)
 !10 = distinct !DISubprogram(name: "test_inline_strip_conflict", scope: !1, file: !1, line: 10, unit: !0)
 !11 = !DILocation(line: 11, scope: !10)
 !12 = distinct !DISubprogram(name: "test_noinline", scope: !1, file: !1, line: 12, unit: !0)
 !13 = !DILocation(line: 13, scope: !12)