Handle profile mismatch correctly for SamplePGO.

[danielcdh]

Summary

Fix the bug when promoted call return type mismatches with the promoted function, we should not try to inline it. Otherwise it may lead to compiler crash.

[tejohnson]

lib/Transforms/IPO/SampleProfile.cpp

764

Can this only happen as the result of an ICP? If so, should the check be moved up to where DI is reset after the promotion, and this turned into an assert?

[danielcdh]

lib/Transforms/IPO/SampleProfile.cpp

764

Good point. Done.

[danielcdh]

update

[tejohnson]

LGTM

lib/Transforms/IPO/SampleProfile.cpp

while (true) {
     for (auto I : CIS) {
       InlineFunctionInfo IFI(nullptr, &GetAC);
       Function *CalledFunction = CallSite(I).getCalledFunction();
       // Do not inline recursive calls.
       if (CalledFunction == &F)
         continue;
       Instruction *DI = I;
       if (!CalledFunction && !PromotedInsns.count(I) &&
-          CallSite(I).isIndirectCall())
+          CallSite(I).isIndirectCall()) {
         for (const auto *FS : findIndirectCallFunctionSamples(*I)) {
           auto CalleeFunctionName = FS->getName();
           // If it is a recursive call, we do not inline it as it could bloat
           // the code exponentially. There is way to better handle this, e.g.
           // clone the caller first, and inline the cloned caller if it is
           // recursive. As llvm does not inline recursive calls, we will simply
           // ignore it instead of handling it explicitly.
           if (CalleeFunctionName == F.getName())
for (auto I : CIS) {
             PromotedInsns.insert(I);
           } else {
             DEBUG(dbgs() << "\nFailed to promote indirect call to "
                          << CalleeFunctionName << " because " << Reason
                          << "\n");
             continue;
           }
         }
+        // If there is profile mismatch, we should not attempt to inline DI.
+        if (!isa<CallInst>(DI) && !isa<InvokeInst>(DI))
+          continue;
+      }
       if (!CalledFunction || !CalledFunction->getSubprogram()) {
         findCalleeFunctionSamples(*I)->findImportedFunctions(
             ImportGUIDs, F.getParent(),
             Samples->getTotalSamples() * SampleProfileHotThreshold / 100);
         continue;
       }
+      assert(isa<CallInst>(DI) || isa<InvokeInst>(DI));
       CallSite CS(DI);
       DebugLoc DLoc = I->getDebugLoc();
       BasicBlock *BB = I->getParent();
       InlineParams Params = getInlineParams();
       Params.ComputeFullInlineCost = true;
       // Checks if there is anything in the reachable portion of the callee at
       // this callsite that makes this inlining potentially illegal. Need to
       // set ComputeFullInlineCost, otherwise getInlineCost may return early

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

  1: foo_inline_strip:3000
   1: 3000
 test_inline_strip_conflict:3000:0
  1: foo_inline_strip_conflict:3000
   1: 3000
 test_norecursive_inline:3000:0
  1: test_norecursive_inline:3000
   20: 3000
+test_noinline_bitcast:3000:0
+ 1: foo_direct_i32:3000
+  1: 3000

test/Transforms/SampleProfile/indirect-call.ll

define void @test_noinline(void ()*) !dbg !12 {
   store void ()* %0, void ()** %2
   %3 = load void ()*, void ()** %2
 ; CHECK-NOT: icmp
 ; CHECK: call
   call void %3(), !dbg !13
   ret void
 }
 
+; CHECK-LABEL: @test_noinline_bitcast
+; If the indirect call has been promoted to a direct call with bitcast,
+; do not inline it.
+define float @test_noinline_bitcast(float ()*) !dbg !26 {
+  %2 = alloca float ()*
+  store float ()* %0, float ()** %2
+; CHECK: icmp
+; CHECK: call
+  %3 = load float ()*, float ()** %2
+  %4 = call float %3(), !dbg !27
+  ret float %4
+}
+
 ; CHECK-LABEL: @test_norecursive_inline
 ; If the indirect call target is the caller, we should not promote it.
 define void @test_norecursive_inline() !dbg !24 {
 ; CHECK-NOT: icmp
 ; CHECK: call
   %1 = load void ()*, void ()** @y, align 8
   call void %1(), !dbg !25
   ret void

 define i32 @foo_noinline(i32 %x) !dbg !20 {
   ret i32 %x
 }
 
 define void @foo_direct() !dbg !21 {
   ret void
 }
 
+define i32 @foo_direct_i32() !dbg !28 {
+  ret i32 0;
+}
+
 ; CHECK-LABEL: @test_direct
 ; We should not promote a direct call.
 define void @test_direct() !dbg !22 {
 ; CHECK-NOT: icmp
 ; CHECK: call
   call void @foo_alias(), !dbg !23
   ret void
 }

 !18 = distinct !DISubprogram(name: "foo_inline_strip_conflict.suffix3", scope: !1, file: !1, line: 1, unit: !0)
 !19 = distinct !DISubprogram(name: "foo_inline2", scope: !1, file: !1, line: 19, unit: !0)
 !20 = distinct !DISubprogram(name: "foo_noinline", scope: !1, file: !1, line: 20, unit: !0)
 !21 = distinct !DISubprogram(name: "foo_direct", scope: !1, file: !1, line: 21, unit: !0)
 !22 = distinct !DISubprogram(name: "test_direct", scope: !1, file: !1, line: 22, unit: !0)
 !23 = !DILocation(line: 23, scope: !22)
 !24 = distinct !DISubprogram(name: "test_norecursive_inline", scope: !1, file: !1, line: 12, unit: !0)
 !25 = !DILocation(line: 13, scope: !24)
+!26 = distinct !DISubprogram(name: "test_noinline_bitcast", scope: !1, file: !1, line: 12, unit: !0)
+!27 = !DILocation(line: 13, scope: !26)
+!28 = distinct !DISubprogram(name: "foo_direct_i32", scope: !1, file: !1, line: 11, unit: !0)