Skip to content

[InstrProf] Support conditional counter updates for integer counters #109222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

alanzhao1
Copy link
Contributor

In #102542. conditional counter updates were only implemented for boolean counters. This PR implements conditional counter updates for integer counters. When the flag -conditional-counter-update is set, integer counters are now treated like boolean counters - @llvm.instrprof.update now checks whether or not a counter is zero, and if not, writes 1 to it.

The motiviation for this mode is that we can bring the benefits of conditional single byte counter updates to languages whose frontends do not support single byte counters (e.g. Rust).

In llvm#102542. conditional counter updates were only implemented for
boolean counters. This PR implements conditional counter updates for
integer counters. When the flag -conditional-counter-update is set,
integer counters are now treated like boolean counters -
@llvm.instrprof.update now checks whether or not a counter is zero, and
if not, writes 1 to it.

The motiviation for this mode is that we can bring the benefits of
conditional single byte counter updates to languages whose frontends do
not support single byte counters (e.g. Rust).
@llvmbot llvmbot added PGO Profile Guided Optimizations llvm:transforms labels Sep 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 19, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-pgo

Author: Alan Zhao (alanzhao1)

Changes

In #102542. conditional counter updates were only implemented for boolean counters. This PR implements conditional counter updates for integer counters. When the flag -conditional-counter-update is set, integer counters are now treated like boolean counters - @llvm.instrprof.update now checks whether or not a counter is zero, and if not, writes 1 to it.

The motiviation for this mode is that we can bring the benefits of conditional single byte counter updates to languages whose frontends do not support single byte counters (e.g. Rust).


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp (+14-2)
  • (modified) llvm/test/Instrumentation/InstrProfiling/conditional-counter-updates.ll (+91)
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index 014e049ed0d8e5..9d5394deda0856 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -1263,8 +1263,20 @@ void InstrLowerer::lowerIncrement(InstrProfIncrementInst *Inc) {
                             MaybeAlign(), AtomicOrdering::Monotonic);
   } else {
     Value *IncStep = Inc->getStep();
-    Value *Load = Builder.CreateLoad(IncStep->getType(), Addr, "pgocount");
-    auto *Count = Builder.CreateAdd(Load, Inc->getStep());
+    auto *CtrTy = IncStep->getType();
+    Value *Load = Builder.CreateLoad(CtrTy, Addr, "pgocount");
+    Value *Count;
+
+    if (ConditionalCounterUpdate) {
+      Instruction *SplitBefore = Inc->getNextNode();
+      Value *Cmp = Builder.CreateIsNull(Load, "pgocount.ifzero");
+      Instruction *ThenBranch =
+          SplitBlockAndInsertIfThen(Cmp, SplitBefore, false);
+      Builder.SetInsertPoint(ThenBranch);
+      Count = ConstantInt::get(CtrTy, 1);
+    } else {
+      Count = Builder.CreateAdd(Load, Inc->getStep());
+    }
     auto *Store = Builder.CreateStore(Count, Addr);
     if (isCounterPromotionEnabled())
       PromotionCandidates.emplace_back(cast<Instruction>(Load), Store);
diff --git a/llvm/test/Instrumentation/InstrProfiling/conditional-counter-updates.ll b/llvm/test/Instrumentation/InstrProfiling/conditional-counter-updates.ll
index 998443ec0cd568..ebd510cfb5f08a 100644
--- a/llvm/test/Instrumentation/InstrProfiling/conditional-counter-updates.ll
+++ b/llvm/test/Instrumentation/InstrProfiling/conditional-counter-updates.ll
@@ -5,6 +5,9 @@ target triple = "x86_64-unknown-linux-gnu"
 @__profn_foo = private constant [3 x i8] c"foo"
 @__profn_bar = private constant [3 x i8] c"bar"
 
+@__profn_fooint = private constant [6 x i8] c"fooint"
+@__profn_barint = private constant [6 x i8] c"barint"
+
 ; CHECK-LABEL: define void @foo
 ; CHECK-NEXT: %pgocount = load i8, ptr @__profc_foo, align 1
 ; CHECK-NEXT: %pgocount.ifnonzero = icmp ne i8 %pgocount, 0
@@ -67,3 +70,91 @@ return:                                           ; preds = %if.end, %if.then
   %1 = load i32, ptr %retval, align 4
   ret i32 %1
 }
+
+; CHECK-LABEL: define void @fooint
+; CHECK-NEXT: %pgocount = load i64, ptr @__profc_fooint, align 8
+; CHECK-NEXT: %pgocount.ifzero = icmp eq i64 %pgocount, 0
+; CHECK-NEXT: br i1 %pgocount.ifzero, label %1, label %2
+
+; CHECK-LABEL: 1:
+; CHECK-NEXT: store i64 1, ptr @__profc_fooint, align 8
+; CHECK-NEXT: br label %2
+
+; CHECK-LABEL: 2:
+; CHECK-NEXT: ret void
+define void @fooint() {
+  call void @llvm.instrprof.increment(ptr @__profn_fooint, i64 0, i32 1, i32 0)
+  ret void
+}
+
+; CHECK-LABEL: define i32 @barint
+; CHECK-LABEL: entry:
+; CHECK-NEXT: %retval = alloca i32, align 4
+; CHECK-NEXT: %cond.addr = alloca i32, align 4
+; CHECK-NEXT: store i32 %cond, ptr %cond.addr, align 4
+; CHECK-NEXT: %pgocount = load i64, ptr @__profc_barint, align 8
+; CHECK-NEXT: %pgocount.ifzero = icmp eq i64 %pgocount, 0
+; CHECK-NEXT: br i1 %pgocount.ifzero, label %0, label %1
+
+; CHECK-LABEL: 0:                                  ; preds = %entry
+; CHECK-NEXT: store i64 1, ptr @__profc_barint, align 8
+; CHECK-NEXT: br label %1
+
+; CHECK-LABEL: 1:                                  ; preds = %entry, %0
+; CHECK-NEXT: %2 = load i32, ptr %cond.addr, align 4
+; CHECK-NEXT: %cmp = icmp slt i32 %2, 0
+; CHECK-NEXT: br i1 %cmp, label %if.then, label %if.end
+
+; CHECK-LABEL: if.then:                            ; preds = %1
+; CHECK-NEXT: %pgocount1 = load i64, ptr getelementptr inbounds ([3 x i64], ptr @__profc_barint, i32 0, i32 1), align 8
+; CHECK-NEXT: %pgocount.ifzero2 = icmp eq i64 %pgocount1, 0
+; CHECK-NEXT: br i1 %pgocount.ifzero2, label %3, label %4
+
+; CHECK-LABEL: 3:                                                ; preds = %if.then
+; CHECK-NEXT: store i64 1, ptr getelementptr inbounds ([3 x i64], ptr @__profc_barint, i32 0, i32 1), align 8
+; CHECK-NEXT: br label %4
+; 
+; CHECK-LABEL: 4:                                                ; preds = %if.then, %3
+; CHECK-NEXT: store i32 -1, ptr %retval, align 4
+; CHECK-NEXT: br label %return
+; 
+; CHECK-LABEL: if.end:                                           ; preds = %1
+; CHECK-NEXT: %pgocount3 = load i64, ptr getelementptr inbounds ([3 x i64], ptr @__profc_barint, i32 0, i32 2), align 8
+; CHECK-NEXT: %pgocount.ifzero4 = icmp eq i64 %pgocount3, 0
+; CHECK-NEXT: br i1 %pgocount.ifzero4, label %5, label %6
+; 
+; CHECK-LABEL: 5:                                                ; preds = %if.end
+; CHECK-NEXT: store i64 1, ptr getelementptr inbounds ([3 x i64], ptr @__profc_barint, i32 0, i32 2), align 8
+; CHECK-NEXT: br label %6
+; 
+; CHECK-LABEL: 6:                                                ; preds = %if.end, %5
+; CHECK-NEXT: store i32 0, ptr %retval, align 4
+; CHECK-NEXT: br label %return
+; 
+; CHECK-LABEL: return:                                           ; preds = %6, %4
+; CHECK-NEXT: %7 = load i32, ptr %retval, align 4
+; CHECK-NEXT: ret i32 %7
+define i32 @barint(i32 %cond) #0 {
+entry:
+  %retval = alloca i32, align 4
+  %cond.addr = alloca i32, align 4
+  store i32 %cond, ptr %cond.addr, align 4
+  call void @llvm.instrprof.increment(ptr @__profn_barint, i64 0, i32 3, i32 0)
+  %0 = load i32, ptr %cond.addr, align 4
+  %cmp = icmp slt i32 %0, 0
+  br i1 %cmp, label %if.then, label %if.end
+
+if.then:                                          ; preds = %entry
+  call void @llvm.instrprof.increment(ptr @__profn_barint, i64 0, i32 3, i32 1)
+  store i32 -1, ptr %retval, align 4
+  br label %return
+
+if.end:                                           ; preds = %entry
+  call void @llvm.instrprof.increment(ptr @__profn_barint, i64 0, i32 3, i32 2)
+  store i32 0, ptr %retval, align 4
+  br label %return
+
+return:                                           ; preds = %if.end, %if.then
+  %1 = load i32, ptr %retval, align 4
+  ret i32 %1
+}
\ No newline at end of file

@alanzhao1
Copy link
Contributor Author

I'm going to close this as I just realized this won't work.

Say for example we have

foo() // counter 0

if (...)
  bar(); // counter 1
else;
  baz(); // counter 0 - 1

Clang doesn't insert counters for each branch of an if statement - it only inserts a counter for one branch (in this case, bar()) and infers the coverage of the second branch by subtracting the counter for the first branch from the counter of the preceding basic block. Therefore, if we apply any cap to the counter values, and foo(), bar(), and baz() are all called, the coverage report will incorrectly report that baz() was never called because counter 0 (with a value of 1) - counter 1 (also with a value of 1) = 0.

@alanzhao1 alanzhao1 closed this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:transforms PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants