[SampleFDO] Fix invalid branch profile generated by indirect call promotion

[wmi]

Summary

Suppose an inline instance has hot total sample count but 0 entry count, and it is an indirect call target. If the indirect call has no other call target and inline instance associated with it and it is promoted, currently the conditional branch generated by indirect call promotion will have invalid branch profile which is !{!"branch_weights", i32 0, i32 0} -- because the entry count of the promoted target is 0 and the total entry count of all targets is also 0. This caused a SEGV in Control Height Reduction and may cause problem in other passes.

Function entry count of an inline instance is computed by a heuristic -- using either the sample of the starting line or starting inner inline instance. The patch changes the heuristic a little bit so that when total sample count is larger than 0, the computed entry count will be at least 1. Then the new branch profile will be !{!"branch_weights", i32 1, i32 0}.

[davidxl]

lgtm

llvm/include/llvm/ProfileData/SampleProf.h

public:
   /// instruction of the symbol. But as we directly get this info for raw
   /// profile without referring to potentially inaccurate debug info, this
   /// gives more accurate profile data and is preferred for standalone symbols.
   uint64_t getHeadSamples() const { return TotalHeadSamples; }
 
   /// Return the sample count of the first instruction of the function.
   /// The function can be either a standalone symbol or an inlined function.
   uint64_t getEntrySamples() const {
+    uint64_t Count = 0;
     // Use either BodySamples or CallsiteSamples which ever has the smaller
     // lineno.
     if (!BodySamples.empty() &&
         (CallsiteSamples.empty() ||
          BodySamples.begin()->first < CallsiteSamples.begin()->first))
-      return BodySamples.begin()->second.getSamples();
-    if (!CallsiteSamples.empty()) {
-      uint64_t T = 0;
+      Count = BodySamples.begin()->second.getSamples();
+    else if (!CallsiteSamples.empty()) {
       // An indirect callsite may be promoted to several inlined direct calls.
       // We need to get the sum of them.
       for (const auto &N_FS : CallsiteSamples.begin()->second)
-        T += N_FS.second.getEntrySamples();
-      return T;
+        Count += N_FS.second.getEntrySamples();
     }
-    return 0;
+    // Return at least 1 if total sample is not 0.
+    return Count ? Count : TotalSamples > 0;
   }
 
   /// Return all the samples collected in the body of the function.
   const BodySampleMap &getBodySamples() const { return BodySamples; }
 
   /// Return all the callsite samples collected in the body of the function.
   const CallsiteSampleMap &getCallsiteSamples() const {
     return CallsiteSamples;

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

 test_noinline_bitcast:3000:0
  1: foo_direct_i32:3000
   1: 3000
 return_arg_caller:3000:0
  1: foo_inline1:3000
   11: 3000
  2: return_arg:3000
   1: 3000
+branch_prof_valid:4000:0
+ 1: 1000
+ 2: foo_inline3:3000
+  1: 0
+  2: 3000

llvm/test/Transforms/SampleProfile/indirect-call.ll

 then:
   %3 = tail call i32* %0(i32* %2), !dbg !32
   ret i32* %3
 
 else:
   ret i32* null
 }
 
+; CHECK-LABEL: @branch_prof_valid
+; Check the conditional branch generated by indirect call promotion won't
+; have invalid profile like !{!"branch_weights", i32 0, i32 0}.
+define void @branch_prof_valid(void ()* %t0) !dbg !33 {
+  %t1 = alloca void ()*
+  store void ()* %t0, void ()** %t1
+  %t2 = load void ()*, void ()** %t1
+  ; CHECK-NOT: call {{.*}}
+  ; CHECK: br i1 {{.*}}, label %if.true.direct_targ, label %if.false.orig_indirect, {{.*}}, !prof ![[BR3:[0-9]+]]
+  call void %t2(), !dbg !34
+  ret void
+}
+
 @x = global i32 0, align 4
 @y = global void ()* null, align 8
 
 define i32* @foo_inline1(i32* %x) !dbg !14 {
   ret i32* %x
 }
 
 define i32* @foo_inline_strip.suffix(i32* %x) !dbg !15 {

 define i32* @foo_inline_strip_conflict.suffix3(i32* %x) !dbg !18 {
   ret i32* %x
 }
 
 define i32* @foo_inline2(i32* %x) !dbg !19 {
   ret i32* %x
 }
 
+define void @foo_inline3() !dbg !35 {
+  ret void
+}
+
 define i32 @foo_noinline(i32 %x) !dbg !20 {
   ret i32 %x
 }
 
 define void @foo_direct() !dbg !21 {
   ret void
 }
 

 !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: ![[BR1]] = !{!"branch_weights", i32 4000, i32 4000}
 ; CHECK: ![[BR2]] = !{!"branch_weights", i32 3000, i32 1000}
 ; CHECK: ![[VP]] = !{!"VP", i32 0, i64 8000, i64 -6391416044382067764, i64 1000}
+; CHECK: ![[BR3]] = !{!"branch_weights", i32 1, i32 0}
 !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)

 !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)
 !29 = distinct !DISubprogram(name: "return_arg", scope: !1, file: !1, line: 11, unit: !0)
 !30 = distinct !DISubprogram(name: "return_arg_caller", scope: !1, file: !1, line: 11, unit: !0)
 !31 = !DILocation(line: 12, scope: !30)
 !32 = !DILocation(line: 13, scope: !30)
+!33 = distinct !DISubprogram(name: "branch_prof_valid", scope: !1, file: !1, line: 25, unit: !0)
+!34 = !DILocation(line: 27, scope: !33)
+!35 = distinct !DISubprogram(name: "foo_inline3", scope: !1, file: !1, line: 29, unit: !0)

llvm/test/Transforms/SampleProfile/inline-callee-update.ll

 ; Make sure Import GUID list for ThinLTO properly maintained while update function's entry count for inlining
 
 ; RUN: opt < %s -passes='thinlto-pre-link<O2>' -pgo-kind=pgo-sample-use-pipeline -sample-profile-file=%S/Inputs/inline-callee-update.prof -S | FileCheck %s
 
 @y = global i32* ()* null, align 8
 @z = global i32* ()* null, align 8
 
+; CHECK: define i32* @sample_loader_inlinee() {{.*}} !prof ![[ENTRY:[0-9]+]]
 define i32* @sample_loader_inlinee() !dbg !3 {
 bb:
   %tmp = call i32* @direct_leaf_func(i32* null), !dbg !4
   %cmp = icmp ne i32* %tmp, null
   br i1 %cmp, label %then, label %else
 
 then:                                             ; preds = %bb
   %tmp1 = load i32* ()*, i32* ()** @z, align 8, !dbg !5
   %tmp2 = call i32* %tmp1(), !dbg !5
   ret i32* %tmp2
 
 else:                                             ; preds = %bb
   ret i32* null
 }
 
+; CHECK: define i32* @cgscc_inlinee() {{.*}} !prof ![[ENTRY:[0-9]+]]
 define i32* @cgscc_inlinee() !dbg !6 {
 bb:
   %tmp = call i32* @direct_leaf_func(i32* null), !dbg !7
   %cmp = icmp ne i32* %tmp, null
   br i1 %cmp, label %then, label %else
 
 then:                                             ; preds = %bb
   %tmp1 = load i32* ()*, i32* ()** @y, align 8, !dbg !8

 !7 = !DILocation(line: 32, scope: !6)
 !8 = !DILocation(line: 33, scope: !6)
 !9 = distinct !DISubprogram(name: "test_sample_loader_inline", scope: !1, file: !1, line: 3, spFlags: DISPFlagDefinition, unit: !0)
 !10 = !DILocation(line: 4, scope: !9)
 !11 = distinct !DISubprogram(name: "test_cgscc_inline", scope: !1, file: !1, line: 20, spFlags: DISPFlagDefinition, unit: !0)
 !12 = !DILocation(line: 21, scope: !11)
 
 ; Make sure the ImportGUID stays with entry count metadata for ThinLTO-PreLink
-; CHECK: distinct !DISubprogram(name: "sample_loader_inlinee"
-; CHECK-NEXT: {!"function_entry_count", i64 1, i64 -9171813444624716006}
-; CHECK: distinct !DISubprogram(name: "cgscc_inlinee"
-; CHECK-NEXT: !{!"function_entry_count", i64 0, i64 -9171813444624716006}
+; CHECK: ![[ENTRY]] = !{!"function_entry_count", i64 1, i64 -9171813444624716006}