[llvm] 1dd39b1 - [PGO] Fix incorrect function entry count

[Rong Xu via llvm-commits]

Author: Rong Xu
Date: 2020-07-24T17:39:55-07:00
New Revision: 1dd39b1133136c58847d9f2d3c73d2ad8e76e25d

URL: https://github.com/llvm/llvm-project/commit/1dd39b1133136c58847d9f2d3c73d2ad8e76e25d
DIFF: https://github.com/llvm/llvm-project/commit/1dd39b1133136c58847d9f2d3c73d2ad8e76e25d.diff

LOG: [PGO] Fix incorrect function entry count

Function entry count might be zero after the profile counts reset and
before reentry to the function.

Zero profile entry count is very bad as the profile count from BFI will
be wrong.

A simple fix is to set the profile entry count to 1 if there are
non-zero profile counts in this function.

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

Added: 
    llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
    llvm/test/Transforms/PGOProfile/fix_entry_count.ll

Modified: 
    llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index 67bf264e8eea..c4a43abaa53c 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -1131,11 +1131,18 @@ bool PGOUseFunc::setInstrumentedCounts(
   if (NumCounters != CountFromProfile.size()) {
     return false;
   }
+  auto *FuncEntry = &*F.begin();
+
   // Set the profile count to the Instrumented BBs.
   uint32_t I = 0;
   for (BasicBlock *InstrBB : InstrumentBBs) {
     uint64_t CountValue = CountFromProfile[I++];
     UseBBInfo &Info = getBBInfo(InstrBB);
+    // If we reach here, we know that we have some nonzero count
+    // values in this function. The entry count should not be 0.
+    // Fix it if necessary.
+    if (InstrBB == FuncEntry && CountValue == 0)
+      CountValue = 1;
     Info.setBBInfoCount(CountValue);
   }
   ProfileCountSize = CountFromProfile.size();
@@ -1326,7 +1333,6 @@ void PGOUseFunc::populateCounters() {
   }
 #endif
   uint64_t FuncEntryCount = getBBInfo(&*F.begin()).CountValue;
-  F.setEntryCount(ProfileCount(FuncEntryCount, Function::PCT_Real));
   uint64_t FuncMaxCount = FuncEntryCount;
   for (auto &BB : F) {
     auto BI = findBBInfo(&BB);
@@ -1334,6 +1340,11 @@ void PGOUseFunc::populateCounters() {
       continue;
     FuncMaxCount = std::max(FuncMaxCount, BI->CountValue);
   }
+
+  // Fix the obviously inconsistent entry count.
+  if (FuncMaxCount > 0 && FuncEntryCount == 0)
+    FuncEntryCount = 1;
+  F.setEntryCount(ProfileCount(FuncEntryCount, Function::PCT_Real));
   markFunctionAttributes(FuncEntryCount, FuncMaxCount);
 
   // Now annotate select instructions

diff  --git a/llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext b/llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
new file mode 100644
index 000000000000..de4771beaacc
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
@@ -0,0 +1,8 @@
+:ir
+:entry_first
+test_simple_for
+34137660316
+2
+0
+96
+

diff  --git a/llvm/test/Transforms/PGOProfile/fix_entry_count.ll b/llvm/test/Transforms/PGOProfile/fix_entry_count.ll
new file mode 100644
index 000000000000..5923bc923a6a
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/fix_entry_count.ll
@@ -0,0 +1,34 @@
+; RUN: llvm-profdata merge %S/Inputs/fix_entry_count.proftext -o %t.profdata
+; RUN: opt < %s -pgo-instr-use -pgo-instrument-entry=true -pgo-test-profile-file=%t.profdata -S | FileCheck %s --check-prefix=USE
+; RUN: opt < %s -passes=pgo-instr-use -pgo-instrument-entry=true -pgo-test-profile-file=%t.profdata -S | FileCheck %s --check-prefix=USE
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @test_simple_for(i32 %n) {
+; USE: define i32 @test_simple_for(i32 %n)
+; USE-SAME: !prof ![[ENTRY_COUNT:[0-9]*]]
+entry:
+  br label %for.cond
+
+for.cond:
+  %i = phi i32 [ 0, %entry ], [ %inc1, %for.inc ]
+  %sum = phi i32 [ 1, %entry ], [ %inc, %for.inc ]
+  %cmp = icmp slt i32 %i, %n
+  br i1 %cmp, label %for.body, label %for.end
+; USE: br i1 %cmp, label %for.body, label %for.end
+; USE-SAME: !prof ![[BW_FOR_COND:[0-9]+]]
+
+for.body:
+  %inc = add nsw i32 %sum, 1
+  br label %for.inc
+
+for.inc:
+  %inc1 = add nsw i32 %i, 1
+  br label %for.cond
+
+for.end:
+  ret i32 %sum
+}
+; USE: ![[ENTRY_COUNT]] = !{!"function_entry_count", i64 1}
+; USE: ![[BW_FOR_COND]] = !{!"branch_weights", i32 96, i32 1}