[InstrProf] Add pgo use block coverage test

[ellishg]

Back in https://reviews.llvm.org/D124490 we added a block coverage mode that instruments a subset of basic blocks using single byte counters to get coverage for the whole function.

This commit adds a test to make sure that we correctly assign branch weights based on the coverage profile.

I noticed this test was missing after seeing that we had no coverage on PGOUseFunc::populateCoverage()
https://lab.llvm.org/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp.html#L1383

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-pgo

Author: Ellis Hoag (ellishg)

Changes

Back in https://reviews.llvm.org/D124490 we added a block coverage mode that instruments a subset of basic blocks using single byte counters to get coverage for the whole function.

This commit adds a test to make sure that we correctly assign branch weights based on the coverage profile.

I noticed this test was missing after seeing that we had no coverage on PGOUseFunc::populateCoverage()
https://lab.llvm.org/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp.html#L1383

---

Full diff: https://github.com/llvm/llvm-project/pull/72443.diff

4 Files Affected:

• (modified) llvm/lib/ProfileData/InstrProfReader.cpp (+2)
• (modified) llvm/lib/ProfileData/InstrProfWriter.cpp (+2)
• (added) llvm/test/Transforms/PGOProfile/Inputs/coverage.proftext (+54)
• (modified) llvm/test/Transforms/PGOProfile/coverage.ll (+28-6)

diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 9d5140da4db248e..fd41d6badf2b34b 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -271,6 +271,8 @@ Error TextInstrProfReader::readHeader() {
       ProfileKind |= InstrProfKind::FunctionEntryInstrumentation;
     else if (Str.equals_insensitive("not_entry_first"))
       ProfileKind &= ~InstrProfKind::FunctionEntryInstrumentation;
+    else if (Str.equals_insensitive("single_byte_coverage"))
+      ProfileKind |= InstrProfKind::SingleByteCoverage;
     else if (Str.equals_insensitive("temporal_prof_traces")) {
       ProfileKind |= InstrProfKind::TemporalProfile;
       if (auto Err = readTemporalProfTraceData())
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 85339b39c74c4c7..595c9aa1adc105e 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -762,6 +762,8 @@ Error InstrProfWriter::writeText(raw_fd_ostream &OS) {
   if (static_cast<bool>(ProfileKind &
                         InstrProfKind::FunctionEntryInstrumentation))
     OS << "# Always instrument the function entry block\n:entry_first\n";
+  if (static_cast<bool>(ProfileKind & InstrProfKind::SingleByteCoverage))
+    OS << "# Instrument block coverage\n:single_byte_coverage\n";
   InstrProfSymtab Symtab;
 
   using FuncPair = detail::DenseMapPair<uint64_t, InstrProfRecord>;
diff --git a/llvm/test/Transforms/PGOProfile/Inputs/coverage.proftext b/llvm/test/Transforms/PGOProfile/Inputs/coverage.proftext
new file mode 100644
index 000000000000000..229530ba414065d
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/Inputs/coverage.proftext
@@ -0,0 +1,54 @@
+:ir
+:single_byte_coverage
+
+foo
+# Func Hash:
+848064302753700500
+# Num Counters:
+2
+# Counter Values:
+3
+4
+
+
+bar
+# Func Hash:
+848064302952419074
+# Num Counters:
+2
+# Counter Values:
+2
+0
+
+
+goo
+# Func Hash:
+1106497858086895615
+# Num Counters:
+1
+# Counter Values:
+5
+
+
+loop
+# Func Hash:
+92940490389974880
+# Num Counters:
+2
+# Counter Values:
+1
+1
+
+
+hoo
+# Func Hash:
+1073332642652768409
+# Num Counters:
+6
+# Counter Values:
+1
+0
+1
+1
+0
+0
diff --git a/llvm/test/Transforms/PGOProfile/coverage.ll b/llvm/test/Transforms/PGOProfile/coverage.ll
index d636be14a2cc99e..1b7658a2ea9ed07 100644
--- a/llvm/test/Transforms/PGOProfile/coverage.ll
+++ b/llvm/test/Transforms/PGOProfile/coverage.ll
@@ -1,14 +1,20 @@
-; RUN: opt < %s -passes=pgo-instr-gen -pgo-function-entry-coverage -S | FileCheck %s --implicit-check-not="instrprof.cover" --check-prefixes=CHECK,ENTRY
-; RUN: opt < %s -passes=pgo-instr-gen -pgo-block-coverage -S | FileCheck %s --implicit-check-not="instrprof.cover" --check-prefixes=CHECK,BLOCK
+; RUN: opt < %s -passes=pgo-instr-gen -pgo-function-entry-coverage -S | FileCheck %s --implicit-check-not="instrprof.cover" --check-prefixes=CHECK,GEN,ENTRY
+; RUN: opt < %s -passes=pgo-instr-gen -pgo-block-coverage -S | FileCheck %s --implicit-check-not="instrprof.cover" --check-prefixes=CHECK,GEN,BLOCK
+
+; RUN: llvm-profdata merge %S/Inputs/coverage.proftext -o %t.profdata
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s --check-prefixes=CHECK,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"
 
+; CHECK-LABEL: @foo()
+; USE-SAME: !prof ![[HOT:[0-9]+]]
 define void @foo() {
 ; CHECK-LABEL: entry:
 entry:
   ; ENTRY: call void @llvm.instrprof.cover({{.*}})
   %c = call i1 @choice()
   br i1 %c, label %if.then, label %if.else
+  ; USE: br i1 %c, label %if.then, label %if.else, !prof ![[WEIGHTS0:[0-9]+]]
 
 ; CHECK-LABEL: if.then:
 if.then:
@@ -25,12 +31,15 @@ if.end:
   ret void
 }
 
+; CHECK-LABEL: @bar()
+; USE-SAME: !prof ![[HOT:[0-9]+]]
 define void @bar() {
 ; CHECK-LABEL: entry:
 entry:
   ; ENTRY: call void @llvm.instrprof.cover({{.*}})
   %c = call i1 @choice()
   br i1 %c, label %if.then, label %if.end
+  ; USE: br i1 %c, label %if.then, label %if.end, !prof ![[WEIGHTS1:[0-9]+]]
 
 ; CHECK-LABEL: if.then:
 if.then:
@@ -43,24 +52,29 @@ if.end:
   ret void
 }
 
+; CHECK-LABEL: @goo()
+; USE-SAME: !prof ![[HOT:[0-9]+]]
 define void @goo() {
 ; CHECK-LABEL: entry:
 entry:
-  ; CHECK: call void @llvm.instrprof.cover({{.*}})
+  ; GEN: call void @llvm.instrprof.cover({{.*}})
   ret void
 }
 
+; CHECK-LABEL: @loop()
+; USE-SAME: !prof ![[HOT:[0-9]+]]
 define void @loop() {
 ; CHECK-LABEL: entry:
 entry:
-  ; CHECK: call void @llvm.instrprof.cover({{.*}})
+  ; GEN: call void @llvm.instrprof.cover({{.*}})
   br label %while
 while:
   ; BLOCK: call void @llvm.instrprof.cover({{.*}})
   br label %while
 }
 
-; Function Attrs: noinline nounwind ssp uwtable
+; CHECK-LABEL: @hoo(
+; USE-SAME: !prof ![[HOT:[0-9]+]]
 define void @hoo(i32 %a) #0 {
 ; CHECK-LABEL: entry:
 entry:
@@ -72,6 +86,7 @@ entry:
   %rem = srem i32 %0, 2
   %cmp = icmp eq i32 %rem, 0
   br i1 %cmp, label %if.then, label %if.else
+  ; USE: br i1 %cmp, label %if.then, label %if.else, !prof ![[WEIGHTS1]]
 
 ; CHECK-LABEL: if.then:
 if.then:                                          ; preds = %entry
@@ -94,6 +109,7 @@ for.cond:                                         ; preds = %for.inc, %if.end
   %2 = load i32, i32* %a.addr, align 4
   %cmp1 = icmp slt i32 %1, %2
   br i1 %cmp1, label %for.body, label %for.end
+  ; USE: br i1 %cmp1, label %for.body, label %for.end, !prof ![[WEIGHTS1]]
 
 ; CHECK-LABEL: for.body:
 for.body:                                         ; preds = %for.cond
@@ -101,6 +117,7 @@ for.body:                                         ; preds = %for.cond
   %rem2 = srem i32 %3, 3
   %cmp3 = icmp eq i32 %rem2, 0
   br i1 %cmp3, label %if.then4, label %if.else5
+  ; USE: br i1 %cmp3, label %if.then4, label %if.else5, !prof ![[WEIGHTS0]]
 
 ; CHECK-LABEL: if.then4:
 if.then4:                                         ; preds = %for.body
@@ -113,6 +130,7 @@ if.else5:                                         ; preds = %for.body
   %rem6 = srem i32 %4, 1001
   %cmp7 = icmp eq i32 %rem6, 0
   br i1 %cmp7, label %if.then8, label %if.end9
+  ; USE: br i1 %cmp7, label %if.then8, label %if.end9, !prof ![[WEIGHTS1]]
 
 ; CHECK-LABEL: if.then8:
 if.then8:                                         ; preds = %if.else5
@@ -147,4 +165,8 @@ return:                                           ; preds = %for.end, %if.then8
 
 declare i1 @choice()
 
-; CHECK: declare void @llvm.instrprof.cover({{.*}})
+; GEN: declare void @llvm.instrprof.cover({{.*}})
+
+; USE-DAG: ![[HOT]] = !{!"function_entry_count", i64 10000}
+; USE-DAG: ![[WEIGHTS0]] = !{!"branch_weights", i32 1, i32 1}
+; USE-DAG: ![[WEIGHTS1]] = !{!"branch_weights", i32 1, i32 0}

[snehasish]

lgtm

[snehasish]

Does --check-prefixes need the "CHECK" item now? It looks like the uses were replaced by GEN. And the other strings that match in this file are CHECK-LABEL directives.

(Same for the other run commands)

[ellishg]

I think the CHECKs are still useful. Yes, they are only used for CHECK-LABELS, but those are shared by all the run commands, so it helps organize the checks.