[llvm] 154cd6d - [SampleFDO] Fix invalid branch profile generated by indirect call promotion.

Wei Mi via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 15 18:37:16 PST 2020


Author: Wei Mi
Date: 2020-01-15T18:36:06-08:00
New Revision: 154cd6de513e1e9ce794ba2d1eae1647c873f812

URL: https://github.com/llvm/llvm-project/commit/154cd6de513e1e9ce794ba2d1eae1647c873f812
DIFF: https://github.com/llvm/llvm-project/commit/154cd6de513e1e9ce794ba2d1eae1647c873f812.diff

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

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}.

Differential Revision: https://reviews.llvm.org/D72790

Added: 
    

Modified: 
    llvm/include/llvm/ProfileData/SampleProf.h
    llvm/test/Transforms/SampleProfile/Inputs/indirect-call.compact.afdo
    llvm/test/Transforms/SampleProfile/Inputs/indirect-call.prof
    llvm/test/Transforms/SampleProfile/indirect-call.ll
    llvm/test/Transforms/SampleProfile/inline-callee-update.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index f8be89c569b7..d3ba55918a2d 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -416,21 +416,21 @@ class FunctionSamples {
   /// 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.

diff  --git a/llvm/test/Transforms/SampleProfile/Inputs/indirect-call.compact.afdo b/llvm/test/Transforms/SampleProfile/Inputs/indirect-call.compact.afdo
index 579f03c85158..b08efbecf55f 100644
Binary files a/llvm/test/Transforms/SampleProfile/Inputs/indirect-call.compact.afdo and b/llvm/test/Transforms/SampleProfile/Inputs/indirect-call.compact.afdo 
diff er

diff  --git a/llvm/test/Transforms/SampleProfile/Inputs/indirect-call.prof b/llvm/test/Transforms/SampleProfile/Inputs/indirect-call.prof
index 5cbfc0a73bcd..af52f01febd5 100644
--- a/llvm/test/Transforms/SampleProfile/Inputs/indirect-call.prof
+++ b/llvm/test/Transforms/SampleProfile/Inputs/indirect-call.prof
@@ -29,3 +29,8 @@ return_arg_caller:3000:0
   11: 3000
  2: return_arg:3000
   1: 3000
+branch_prof_valid:4000:0
+ 1: 1000
+ 2: foo_inline3:3000
+  1: 0
+  2: 3000

diff  --git a/llvm/test/Transforms/SampleProfile/indirect-call.ll b/llvm/test/Transforms/SampleProfile/indirect-call.ll
index 95d0c473ae7e..3249cf5f185b 100644
--- a/llvm/test/Transforms/SampleProfile/indirect-call.ll
+++ b/llvm/test/Transforms/SampleProfile/indirect-call.ll
@@ -121,6 +121,19 @@ 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
 
@@ -148,6 +161,10 @@ 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
 }
@@ -184,6 +201,7 @@ define void @test_direct() !dbg !22 {
 ; 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)
@@ -211,3 +229,6 @@ define void @test_direct() !dbg !22 {
 !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)

diff  --git a/llvm/test/Transforms/SampleProfile/inline-callee-update.ll b/llvm/test/Transforms/SampleProfile/inline-callee-update.ll
index 6001d7741724..36f13eae5539 100644
--- a/llvm/test/Transforms/SampleProfile/inline-callee-update.ll
+++ b/llvm/test/Transforms/SampleProfile/inline-callee-update.ll
@@ -5,6 +5,7 @@
 @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
@@ -20,6 +21,7 @@ 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
@@ -67,7 +69,4 @@ declare i32* @direct_leaf_func(i32*)
 !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}


        


More information about the llvm-commits mailing list