Skip to content

[Coverage][Single] Enable Branch coverage for IfStmt #113111

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

Open
wants to merge 54 commits into
base: users/chapuni/cov/single/nextcount
Choose a base branch
from

Conversation

chapuni
Copy link
Contributor

@chapuni chapuni commented Oct 21, 2024

`SingleByteCoverage` is not per-region attribute at least.
At the moment, this change moves it into `FunctionRecord`.
- Round `Counts` as 1/0
- Confirm both `ExecutionCount` and `AltExecutionCount` are in range.
…/single/nextcount' into users/chapuni/cov/single/base
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Oct 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-clang

Author: NAKAMURA Takumi (chapuni)

Changes

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

4 Files Affected:

  • (modified) clang/lib/CodeGen/CGStmt.cpp (+13-18)
  • (modified) clang/lib/CodeGen/CodeGenPGO.cpp (-12)
  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+5-16)
  • (modified) clang/test/CoverageMapping/single-byte-counters.cpp (+20-16)
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index dbc1ce9bf993cd..c511e5f4f4213a 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -840,8 +840,7 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
     // If the skipped block has no labels in it, just emit the executed block.
     // This avoids emitting dead code and simplifies the CFG substantially.
     if (S.isConstexpr() || !ContainsLabel(Skipped)) {
-      if (CondConstant)
-        incrementProfileCounter(&S);
+      incrementProfileCounter(!CondConstant, &S, true);
       if (Executed) {
         RunCleanupsScope ExecutedScope(*this);
         EmitStmt(Executed);
@@ -851,14 +850,14 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
     }
   }
 
+  auto HasSkip = getIsCounterPair(&S);
+
   // Otherwise, the condition did not fold, or we couldn't elide it.  Just emit
   // the conditional branch.
   llvm::BasicBlock *ThenBlock = createBasicBlock("if.then");
   llvm::BasicBlock *ContBlock = createBasicBlock("if.end");
-  llvm::BasicBlock *ElseBlock = ContBlock;
-  if (Else)
-    ElseBlock = createBasicBlock("if.else");
-
+  llvm::BasicBlock *ElseBlock =
+      (Else || HasSkip.second ? createBasicBlock("if.else") : ContBlock);
   // Prefer the PGO based weights over the likelihood attribute.
   // When the build isn't optimized the metadata isn't used, so don't generate
   // it.
@@ -891,10 +890,7 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
 
   // Emit the 'then' code.
   EmitBlock(ThenBlock);
-  if (llvm::EnableSingleByteCoverage)
-    incrementProfileCounter(S.getThen());
-  else
-    incrementProfileCounter(&S);
+  incrementProfileCounter(false, &S);
   {
     RunCleanupsScope ThenScope(*this);
     EmitStmt(S.getThen());
@@ -908,9 +904,9 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
       auto NL = ApplyDebugLocation::CreateEmpty(*this);
       EmitBlock(ElseBlock);
     }
-    // When single byte coverage mode is enabled, add a counter to else block.
-    if (llvm::EnableSingleByteCoverage)
-      incrementProfileCounter(Else);
+    // Add a counter to else block unless it has CounterExpr.
+    if (HasSkip.second)
+      incrementProfileCounter(true, &S);
     {
       RunCleanupsScope ElseScope(*this);
       EmitStmt(Else);
@@ -920,15 +916,14 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
       auto NL = ApplyDebugLocation::CreateEmpty(*this);
       EmitBranch(ContBlock);
     }
+  } else if (HasSkip.second) {
+    EmitBlock(ElseBlock);
+    incrementProfileCounter(true, &S);
+    EmitBranch(ContBlock);
   }
 
   // Emit the continuation block for code after the if.
   EmitBlock(ContBlock, true);
-
-  // When single byte coverage mode is enabled, add a counter to continuation
-  // block.
-  if (llvm::EnableSingleByteCoverage)
-    incrementProfileCounter(&S);
 }
 
 bool CodeGenFunction::checkIfLoopMustProgress(const Expr *ControllingExpression,
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 0f2090da47a374..f6b9b5c82952c4 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -366,18 +366,6 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
     if (Hash.getHashVersion() == PGO_HASH_V1)
       return Base::TraverseIfStmt(If);
 
-    // When single byte coverage mode is enabled, add a counter to then and
-    // else.
-    bool NoSingleByteCoverage = !llvm::EnableSingleByteCoverage;
-    for (Stmt *CS : If->children()) {
-      if (!CS || NoSingleByteCoverage)
-        continue;
-      if (CS == If->getThen())
-        CounterMap[If->getThen()] = NextCounter++;
-      else if (CS == If->getElse())
-        CounterMap[If->getElse()] = NextCounter++;
-    }
-
     // Otherwise, keep track of which branch we're in while traversing.
     VisitStmt(If);
 
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index a331d5bc68286b..6c6aecb9994c6b 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -2050,12 +2050,7 @@ struct CounterCoverageMappingBuilder
     extendRegion(S->getCond());
 
     Counter ParentCount = getRegion().getCounter();
-    auto [ThenCount, ElseCount] =
-        (llvm::EnableSingleByteCoverage
-             ? std::make_pair(getRegionCounter(S->getThen()),
-                              (S->getElse() ? getRegionCounter(S->getElse())
-                                            : Counter::getZero()))
-             : getBranchCounterPair(S, ParentCount));
+    auto [ThenCount, ElseCount] = getBranchCounterPair(S, ParentCount);
 
     // Emitting a counter for the condition makes it easier to interpret the
     // counter for the body when looking at the coverage.
@@ -2080,26 +2075,20 @@ struct CounterCoverageMappingBuilder
         fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ElseCount);
       extendRegion(Else);
 
-      Counter ElseOutCount = propagateCounts(ElseCount, Else);
-      if (!llvm::EnableSingleByteCoverage)
-        OutCount = addCounters(OutCount, ElseOutCount);
+      OutCount = addCounters(OutCount, propagateCounts(ElseCount, Else));
 
       if (ThenHasTerminateStmt)
         HasTerminateStmt = true;
-    } else if (!llvm::EnableSingleByteCoverage)
+    } else
       OutCount = addCounters(OutCount, ElseCount);
 
-    if (llvm::EnableSingleByteCoverage)
-      OutCount = getRegionCounter(S);
-
     if (!IsCounterEqual(OutCount, ParentCount)) {
       pushRegion(OutCount);
       GapRegionCounter = OutCount;
     }
 
-    if (!llvm::EnableSingleByteCoverage)
-      // Create Branch Region around condition.
-      createBranchRegion(S->getCond(), ThenCount, ElseCount);
+    // Create Branch Region around condition.
+    createBranchRegion(S->getCond(), ThenCount, ElseCount);
   }
 
   void VisitCXXTryStmt(const CXXTryStmt *S) {
diff --git a/clang/test/CoverageMapping/single-byte-counters.cpp b/clang/test/CoverageMapping/single-byte-counters.cpp
index d20b695bc2636a..533f791eee19e0 100644
--- a/clang/test/CoverageMapping/single-byte-counters.cpp
+++ b/clang/test/CoverageMapping/single-byte-counters.cpp
@@ -1,36 +1,39 @@
 // RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -mllvm -enable-single-byte-coverage=true -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name single-byte-counters.cpp %s | FileCheck %s
 
 // CHECK: testIf
-int testIf(int x) { // CHECK-NEXT: File 0, [[@LINE]]:19 -> [[@LINE+7]]:2 = [[C00:#0]]
+int testIf(int x) { // CHECK-NEXT: File 0, [[@LINE]]:19 -> [[@LINE+8]]:2 = [[C00:#0]]
   int result = 0;
   if (x == 0)       // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = [[C00]]
-                    // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+1]]:5 = [[C0T:#1]]
+                    // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:13 = [[C0T:#1]], [[C0F:#2]]
+                    // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:14 -> [[@LINE+1]]:5 = [[C0T]]
     result = -1;    // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE]]:16 = [[C0T]]
 
-  return result;    // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE]]:16 = [[C0E:#2]]
+  return result;    // #0
 }
 
 // CHECK-NEXT: testIfElse
-int testIfElse(int x) { // CHECK-NEXT: File 0, [[@LINE]]:23 -> [[@LINE+8]]:2 = [[C10:#0]]
+int testIfElse(int x) { // CHECK-NEXT: File 0, [[@LINE]]:23 -> [[@LINE+9]]:2 = [[C10:#0]]
   int result = 0;
   if (x < 0)            // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:12 = [[C10]]
-                        // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+1]]:5 = [[C1T:#1]]
+                        // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:12 = [[C1T:#1]], [[C1F:#2]]
+                        // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:13 -> [[@LINE+1]]:5 = [[C1T]]
     result = 0;         // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE]]:15 = [[C1T]]
-  else                  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:16 -> [[@LINE+1]]:5 = [[C1F:#2]]
+  else                  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:16 -> [[@LINE+1]]:5 = [[C1F]]
     result = x * x;     // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE]]:19 = [[C1F]]
-  return result;        // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE]]:16 = [[C1E:#3]]
+  return result;        // #0
 }
 
 // CHECK-NEXT: testIfElseReturn
-int testIfElseReturn(int x) { // CHECK-NEXT: File 0, [[@LINE]]:29 -> [[@LINE+9]]:2 = [[C20:#0]]
+int testIfElseReturn(int x) { // CHECK-NEXT: File 0, [[@LINE]]:29 -> [[@LINE+10]]:2 = [[C20:#0]]
   int result = 0;
   if (x > 0)                  // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:12 = [[C20]]
-                              // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+1]]:5 = [[C2T:#1]]
+                              // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:12 = [[C2T:#1]], [[C2F:#2]]
+                              // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:13 -> [[@LINE+1]]:5 = [[C2T]]
     result = x * x;           // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE]]:19 = [[C2T]]
-  else                        // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:20 -> [[@LINE+1]]:5 = [[C2F:#2]]
+  else                        // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:20 -> [[@LINE+1]]:5 = [[C2F]]
     return 0;                 // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE]]:13 = [[C2F]]
-                              // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+1]]:3 = [[C2E:#3]]
-  return result;              // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE]]:16 = [[C2E:#3]]
+                              // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+1]]:3 = [[C2T]]
+  return result;              // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE]]:16 = [[C2T]]
 }
 
 // CHECK-NEXT: testSwitch
@@ -68,16 +71,17 @@ int testWhile() {       // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+11]]:2 =
 }
 
 // CHECK-NEXT: testContinue
-int testContinue() { // CHECK-NEXT: File 0, [[@LINE]]:20 -> [[@LINE+15]]:2 = [[C50:#0]]
+int testContinue() { // CHECK-NEXT: File 0, [[@LINE]]:20 -> [[@LINE+16]]:2 = [[C50:#0]]
   int i = 0;
   int sum = 0;
   while (i < 10) {   // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE]]:16 = [[C5C:#1]]
                      // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:17 -> [[@LINE-1]]:18 = [[C5B:#2]]
-                     // CHECK-NEXT: File 0, [[@LINE-2]]:18 -> [[@LINE+7]]:4 = [[C5B]]
+                     // CHECK-NEXT: File 0, [[@LINE-2]]:18 -> [[@LINE+8]]:4 = [[C5B]]
     if (i == 4)      // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE]]:15 = [[C5B]]
-                     // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:16 -> [[@LINE+1]]:7 = [[C5T:#4]]
+                     // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:9 -> [[@LINE-1]]:15 = [[C5T:#4]], [[C5F:#5]]
+                     // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:16 -> [[@LINE+1]]:7 = [[C5T]]
       continue;      // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:15 = [[C5T]]
-                     // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:16 -> [[@LINE+1]]:5 = [[C5F:#5]]
+                     // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:16 -> [[@LINE+1]]:5 = [[C5F]]
     sum += i;        // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:4 = [[C5F]]
     i++;
   }

@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-clang-codegen

Author: NAKAMURA Takumi (chapuni)

Changes

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

4 Files Affected:

  • (modified) clang/lib/CodeGen/CGStmt.cpp (+13-18)
  • (modified) clang/lib/CodeGen/CodeGenPGO.cpp (-12)
  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+5-16)
  • (modified) clang/test/CoverageMapping/single-byte-counters.cpp (+20-16)
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index dbc1ce9bf993cd..c511e5f4f4213a 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -840,8 +840,7 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
     // If the skipped block has no labels in it, just emit the executed block.
     // This avoids emitting dead code and simplifies the CFG substantially.
     if (S.isConstexpr() || !ContainsLabel(Skipped)) {
-      if (CondConstant)
-        incrementProfileCounter(&S);
+      incrementProfileCounter(!CondConstant, &S, true);
       if (Executed) {
         RunCleanupsScope ExecutedScope(*this);
         EmitStmt(Executed);
@@ -851,14 +850,14 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
     }
   }
 
+  auto HasSkip = getIsCounterPair(&S);
+
   // Otherwise, the condition did not fold, or we couldn't elide it.  Just emit
   // the conditional branch.
   llvm::BasicBlock *ThenBlock = createBasicBlock("if.then");
   llvm::BasicBlock *ContBlock = createBasicBlock("if.end");
-  llvm::BasicBlock *ElseBlock = ContBlock;
-  if (Else)
-    ElseBlock = createBasicBlock("if.else");
-
+  llvm::BasicBlock *ElseBlock =
+      (Else || HasSkip.second ? createBasicBlock("if.else") : ContBlock);
   // Prefer the PGO based weights over the likelihood attribute.
   // When the build isn't optimized the metadata isn't used, so don't generate
   // it.
@@ -891,10 +890,7 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
 
   // Emit the 'then' code.
   EmitBlock(ThenBlock);
-  if (llvm::EnableSingleByteCoverage)
-    incrementProfileCounter(S.getThen());
-  else
-    incrementProfileCounter(&S);
+  incrementProfileCounter(false, &S);
   {
     RunCleanupsScope ThenScope(*this);
     EmitStmt(S.getThen());
@@ -908,9 +904,9 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
       auto NL = ApplyDebugLocation::CreateEmpty(*this);
       EmitBlock(ElseBlock);
     }
-    // When single byte coverage mode is enabled, add a counter to else block.
-    if (llvm::EnableSingleByteCoverage)
-      incrementProfileCounter(Else);
+    // Add a counter to else block unless it has CounterExpr.
+    if (HasSkip.second)
+      incrementProfileCounter(true, &S);
     {
       RunCleanupsScope ElseScope(*this);
       EmitStmt(Else);
@@ -920,15 +916,14 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
       auto NL = ApplyDebugLocation::CreateEmpty(*this);
       EmitBranch(ContBlock);
     }
+  } else if (HasSkip.second) {
+    EmitBlock(ElseBlock);
+    incrementProfileCounter(true, &S);
+    EmitBranch(ContBlock);
   }
 
   // Emit the continuation block for code after the if.
   EmitBlock(ContBlock, true);
-
-  // When single byte coverage mode is enabled, add a counter to continuation
-  // block.
-  if (llvm::EnableSingleByteCoverage)
-    incrementProfileCounter(&S);
 }
 
 bool CodeGenFunction::checkIfLoopMustProgress(const Expr *ControllingExpression,
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 0f2090da47a374..f6b9b5c82952c4 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -366,18 +366,6 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
     if (Hash.getHashVersion() == PGO_HASH_V1)
       return Base::TraverseIfStmt(If);
 
-    // When single byte coverage mode is enabled, add a counter to then and
-    // else.
-    bool NoSingleByteCoverage = !llvm::EnableSingleByteCoverage;
-    for (Stmt *CS : If->children()) {
-      if (!CS || NoSingleByteCoverage)
-        continue;
-      if (CS == If->getThen())
-        CounterMap[If->getThen()] = NextCounter++;
-      else if (CS == If->getElse())
-        CounterMap[If->getElse()] = NextCounter++;
-    }
-
     // Otherwise, keep track of which branch we're in while traversing.
     VisitStmt(If);
 
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index a331d5bc68286b..6c6aecb9994c6b 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -2050,12 +2050,7 @@ struct CounterCoverageMappingBuilder
     extendRegion(S->getCond());
 
     Counter ParentCount = getRegion().getCounter();
-    auto [ThenCount, ElseCount] =
-        (llvm::EnableSingleByteCoverage
-             ? std::make_pair(getRegionCounter(S->getThen()),
-                              (S->getElse() ? getRegionCounter(S->getElse())
-                                            : Counter::getZero()))
-             : getBranchCounterPair(S, ParentCount));
+    auto [ThenCount, ElseCount] = getBranchCounterPair(S, ParentCount);
 
     // Emitting a counter for the condition makes it easier to interpret the
     // counter for the body when looking at the coverage.
@@ -2080,26 +2075,20 @@ struct CounterCoverageMappingBuilder
         fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ElseCount);
       extendRegion(Else);
 
-      Counter ElseOutCount = propagateCounts(ElseCount, Else);
-      if (!llvm::EnableSingleByteCoverage)
-        OutCount = addCounters(OutCount, ElseOutCount);
+      OutCount = addCounters(OutCount, propagateCounts(ElseCount, Else));
 
       if (ThenHasTerminateStmt)
         HasTerminateStmt = true;
-    } else if (!llvm::EnableSingleByteCoverage)
+    } else
       OutCount = addCounters(OutCount, ElseCount);
 
-    if (llvm::EnableSingleByteCoverage)
-      OutCount = getRegionCounter(S);
-
     if (!IsCounterEqual(OutCount, ParentCount)) {
       pushRegion(OutCount);
       GapRegionCounter = OutCount;
     }
 
-    if (!llvm::EnableSingleByteCoverage)
-      // Create Branch Region around condition.
-      createBranchRegion(S->getCond(), ThenCount, ElseCount);
+    // Create Branch Region around condition.
+    createBranchRegion(S->getCond(), ThenCount, ElseCount);
   }
 
   void VisitCXXTryStmt(const CXXTryStmt *S) {
diff --git a/clang/test/CoverageMapping/single-byte-counters.cpp b/clang/test/CoverageMapping/single-byte-counters.cpp
index d20b695bc2636a..533f791eee19e0 100644
--- a/clang/test/CoverageMapping/single-byte-counters.cpp
+++ b/clang/test/CoverageMapping/single-byte-counters.cpp
@@ -1,36 +1,39 @@
 // RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -mllvm -enable-single-byte-coverage=true -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name single-byte-counters.cpp %s | FileCheck %s
 
 // CHECK: testIf
-int testIf(int x) { // CHECK-NEXT: File 0, [[@LINE]]:19 -> [[@LINE+7]]:2 = [[C00:#0]]
+int testIf(int x) { // CHECK-NEXT: File 0, [[@LINE]]:19 -> [[@LINE+8]]:2 = [[C00:#0]]
   int result = 0;
   if (x == 0)       // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = [[C00]]
-                    // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+1]]:5 = [[C0T:#1]]
+                    // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:13 = [[C0T:#1]], [[C0F:#2]]
+                    // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:14 -> [[@LINE+1]]:5 = [[C0T]]
     result = -1;    // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE]]:16 = [[C0T]]
 
-  return result;    // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE]]:16 = [[C0E:#2]]
+  return result;    // #0
 }
 
 // CHECK-NEXT: testIfElse
-int testIfElse(int x) { // CHECK-NEXT: File 0, [[@LINE]]:23 -> [[@LINE+8]]:2 = [[C10:#0]]
+int testIfElse(int x) { // CHECK-NEXT: File 0, [[@LINE]]:23 -> [[@LINE+9]]:2 = [[C10:#0]]
   int result = 0;
   if (x < 0)            // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:12 = [[C10]]
-                        // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+1]]:5 = [[C1T:#1]]
+                        // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:12 = [[C1T:#1]], [[C1F:#2]]
+                        // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:13 -> [[@LINE+1]]:5 = [[C1T]]
     result = 0;         // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE]]:15 = [[C1T]]
-  else                  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:16 -> [[@LINE+1]]:5 = [[C1F:#2]]
+  else                  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:16 -> [[@LINE+1]]:5 = [[C1F]]
     result = x * x;     // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE]]:19 = [[C1F]]
-  return result;        // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE]]:16 = [[C1E:#3]]
+  return result;        // #0
 }
 
 // CHECK-NEXT: testIfElseReturn
-int testIfElseReturn(int x) { // CHECK-NEXT: File 0, [[@LINE]]:29 -> [[@LINE+9]]:2 = [[C20:#0]]
+int testIfElseReturn(int x) { // CHECK-NEXT: File 0, [[@LINE]]:29 -> [[@LINE+10]]:2 = [[C20:#0]]
   int result = 0;
   if (x > 0)                  // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:12 = [[C20]]
-                              // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+1]]:5 = [[C2T:#1]]
+                              // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:12 = [[C2T:#1]], [[C2F:#2]]
+                              // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:13 -> [[@LINE+1]]:5 = [[C2T]]
     result = x * x;           // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE]]:19 = [[C2T]]
-  else                        // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:20 -> [[@LINE+1]]:5 = [[C2F:#2]]
+  else                        // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:20 -> [[@LINE+1]]:5 = [[C2F]]
     return 0;                 // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE]]:13 = [[C2F]]
-                              // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+1]]:3 = [[C2E:#3]]
-  return result;              // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE]]:16 = [[C2E:#3]]
+                              // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+1]]:3 = [[C2T]]
+  return result;              // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE]]:16 = [[C2T]]
 }
 
 // CHECK-NEXT: testSwitch
@@ -68,16 +71,17 @@ int testWhile() {       // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+11]]:2 =
 }
 
 // CHECK-NEXT: testContinue
-int testContinue() { // CHECK-NEXT: File 0, [[@LINE]]:20 -> [[@LINE+15]]:2 = [[C50:#0]]
+int testContinue() { // CHECK-NEXT: File 0, [[@LINE]]:20 -> [[@LINE+16]]:2 = [[C50:#0]]
   int i = 0;
   int sum = 0;
   while (i < 10) {   // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE]]:16 = [[C5C:#1]]
                      // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:17 -> [[@LINE-1]]:18 = [[C5B:#2]]
-                     // CHECK-NEXT: File 0, [[@LINE-2]]:18 -> [[@LINE+7]]:4 = [[C5B]]
+                     // CHECK-NEXT: File 0, [[@LINE-2]]:18 -> [[@LINE+8]]:4 = [[C5B]]
     if (i == 4)      // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE]]:15 = [[C5B]]
-                     // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:16 -> [[@LINE+1]]:7 = [[C5T:#4]]
+                     // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:9 -> [[@LINE-1]]:15 = [[C5T:#4]], [[C5F:#5]]
+                     // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:16 -> [[@LINE+1]]:7 = [[C5T]]
       continue;      // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:15 = [[C5T]]
-                     // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:16 -> [[@LINE+1]]:5 = [[C5F:#5]]
+                     // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:16 -> [[@LINE+1]]:5 = [[C5F]]
     sum += i;        // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:4 = [[C5F]]
     i++;
   }

@@ -864,8 +864,7 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
// If the skipped block has no labels in it, just emit the executed block.
// This avoids emitting dead code and simplifies the CFG substantially.
if (S.isConstexpr() || !ContainsLabel(Skipped)) {
if (CondConstant)
incrementProfileCounter(&S);
incrementProfileCounter(!CondConstant, &S, true);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comment for bool parameter for readability?

e.g. /*varname=*/true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. See also #120930. UseBoth is not classified since I think its usage is rare.

incrementProfileCounter(S.getThen());
else
incrementProfileCounter(&S);
incrementProfileCounter(false, &S);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@ornata
Copy link

ornata commented Dec 23, 2024

Other than a couple nits on comments this looks fine to me.

Conflicts:
	llvm/test/tools/llvm-cov/branch-macros.test
	llvm/test/tools/llvm-cov/showLineExecutionCounts.test
…ingle/if

Conflicts:
	clang/lib/CodeGen/CoverageMappingGen.cpp
Conflicts:
	llvm/test/tools/llvm-cov/branch-macros.test
	llvm/test/tools/llvm-cov/showLineExecutionCounts.test
	llvm/tools/llvm-cov/CodeCoverage.cpp
	llvm/tools/llvm-cov/SourceCoverageView.h
…ingle/if

Conflicts:
	clang/test/CoverageMapping/single-byte-counters.cpp
…ingle/if

Conflicts:
	clang/lib/CodeGen/CoverageMappingGen.cpp
@chapuni chapuni changed the base branch from users/chapuni/cov/single/base to users/chapuni/cov/single/nextcount January 9, 2025 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants