Fix the samplepgo indirect call promotion bug: we should not promote a direct call.

[danielcdh]

Summary

Checking CS.getCalledFunction() == nullptr does not necessary indicate indirect call. We also need to check if CS.getCalledValue() is not a constant.

[danielcdh]

rebase

[davidxl]

lib/Transforms/IPO/SampleProfile.cpp

639

Refactor the code in llvm/Analysis/IndirectCallSiteVisitor.h and reuse the indirect call site query routine.

[danielcdh]

update

[davidxl]

include/llvm/IR/CallSite.h

116 ↗

(On Diff #87275)

check if getCalledValue nullness?

121 ↗

(On Diff #87275)

getCalledValue constant case?

[danielcdh]

update

[danielcdh]

include/llvm/IR/CallSite.h

121 ↗

(On Diff #87275)

I used "stripPointerCasts" on getCalledValue() so that this is not needed any more.

[davidxl]

include/llvm/IR/CallSite.h

121 ↗

(On Diff #87275)

how is it so?

[danielcdh]

include/llvm/IR/CallSite.h

121 ↗

(On Diff #87275)

stripPointerCasts comment says "Strip off pointer casts, all-zero GEPs, and aliases.", so for the constants (alias), it will give the actual function that it aliases to.

[danielcdh]

update

[davidxl]

lgtm.

lib/Transforms/IPO/SampleProfile.cpp

for (auto &BB : F) {
         }
       }
       if (Hot) {
         CIS.insert(CIS.begin(), Candidates.begin(), Candidates.end());
       }
     }
     for (auto I : CIS) {
       InlineFunctionInfo IFI(nullptr, ACT ? &GetAssumptionCache : nullptr);
-      Function *CalledFunction = CallSite(I).getCalledFunction();
+      CallSite CS(I);
+      Function *CalledFunction = CS.getCalledFunction();
       Instruction *DI = I;
-      if (!CalledFunction && !PromotedInsns.count(I)) {
+      if (!CalledFunction && !PromotedInsns.count(I) && CS.getCalledValue() &&
+          !isa<Constant>(CS.getCalledValue())) {
         auto CalleeFunctionName = findCalleeFunctionSamples(*I)->getName();
         const char *Reason = "Callee function not available";
         CalledFunction = F.getParent()->getFunction(CalleeFunctionName);
         if (CalledFunction && isLegalToPromote(I, CalledFunction, &Reason)) {
           // The indirect target was promoted and inlined in the profile, as a
           // result, we do not have profile info for the branch probability.
           // We set the probability to 80% taken to indicate that the static
           // call is likely taken.

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

 test:63067:0
  4: 3345 _Z3barv:1398 _Z3foov:2059
 test_inline:3000:0
  5: foo_inline:3000
   1: 3000
 test_noinline:3000:0
  5: foo_noinline:3000
   1: 3000
+test_direct:3000:0
+ 5: foo_direct:3000
+  1: 3000

test/Transforms/SampleProfile/indirect-call.ll

 define i32* @foo_inline(i32* %x) !dbg !3 {
   ret i32* %x
 }
 
 define i32 @foo_noinline(i32 %x) !dbg !3 {
   ret i32 %x
 }
 
+define void @foo_direct() !dbg !3 {
+  ret void
+}
+
+; CHECK-LABEL: @test_direct
+; We should not promote a direct call.
+define void @test_direct() !dbg !3 {
+; CHECK-NOT: icmp
+; CHECK: call
+  call void @foo_alias(), !dbg !5
+  ret void
+}
+
+@foo_alias = alias void (), void ()* @foo_direct
+
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!2}
 
 !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: 1, unit: !0)
 !4 = !DILocation(line: 5, scope: !3)
 !5 = !DILocation(line: 6, scope: !3)
 ; CHECK: ![[PROF]] = !{!"VP", i32 0, i64 3457, i64 9191153033785521275, i64 2059, i64 -1069303473483922844, i64 1398}