[PATCH] D61540: [PGO] Use sum of count values to fix func entry count and add a check to verify BFI counts

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:257
+
+static cl::opt<unsigned> PGOVerifyBFICutoff(
+    "pgo-verify-bfi-cutoff", cl::init(1), cl::Hidden,
----------------
It is probably more useful to have an option controlling checking hot BB's only -- check with either RawCount or BFI count is hot (can catch the case when a cold block becomes 'hot' with BFI info).


================
Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1559
+// adjust the func entry count.
+static void fixFuncEntryCount(PGOUseFunc &Func, LoopInfo &LI,
+                              BranchProbabilityInfo &NBPI) {
----------------
what is the rationale and use of this fixup? 


================
Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1612
+  bool PrintFunc = false;
+  unsigned BBNum = 0, BBNoMatchNum = 0, NonC0BBNum = 0;
+  for (auto &BBI : F) {
----------------
Add comment of make variable name NonC0BBNum more obvious for its meaning.


================
Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1636
+        dbgs() << "VerifyFuncBFI: in " << F.getName() << "\n";
+        PrintFunc = true;
+      }
----------------
Variable not used.


================
Comment at: llvm/test/Transforms/PGOProfile/fix_bfi.ll:1
+; RUN: llvm-profdata merge %S/Inputs/fix_bfi.proftext -o %t.profdata
+; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-fix-entry-count=true -pgo-verify-bfi-ratio=2 -pgo-verify-bfi=true 2>&1 | FileCheck %s --check-prefix=FIX-CHECK
----------------
Add an comment on the what the test case is testing (especially the fix check part).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61540/new/

https://reviews.llvm.org/D61540