[llvm] [InstrProf] Support conditional counter updates (PR #102542)

via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 8 16:37:14 PDT 2024


================
@@ -1213,8 +1213,20 @@ Value *InstrLowerer::getBitmapAddress(InstrProfMCDCTVBitmapUpdate *I) {
 void InstrLowerer::lowerCover(InstrProfCoverInst *CoverInstruction) {
   auto *Addr = getCounterAddress(CoverInstruction);
   IRBuilder<> Builder(CoverInstruction);
-  // We store zero to represent that this block is covered.
-  Builder.CreateStore(Builder.getInt8(0), Addr);
+  if (ConditionalCounterUpdate) {
+    auto &Ctx = CoverInstruction->getParent()->getContext();
+    auto *Int8Ty = llvm::Type::getInt8Ty(Ctx);
+    Value *Load = Builder.CreateLoad(Int8Ty, Addr, "pgocount");
+    Value *Cmp = Builder.CreateICmpNE(Load, ConstantInt::get(Int8Ty, 0),
+                                      "pgocount.ifnonzero");
+    Value *Sel =
+        Builder.CreateSelect(Cmp, Builder.getInt8(0), Load, "pgocount.select");
----------------
gulfemsavrun wrote:

Ok, that's interesting. Thanks for pointing that out, and I replaced that with a compare and branch.
https://discourse.llvm.org/t/rfc-single-byte-counters-for-source-based-code-coverage/75685/10?u=gulfemsavrun

I realized that you added a break with the following explaination:
```
       } else if (auto *IPC = dyn_cast<InstrProfCoverInst>(&Instr)) {
         lowerCover(IPC);
         MadeChange = true;
+        break; // because this splits and terminates the current basic block
```

Is this really necessary? I did not find it necessary for the simple testing that I did, but am I missing something?

https://github.com/llvm/llvm-project/pull/102542


More information about the llvm-commits mailing list