[llvm-branch-commits] [clang] [Coverage][Single] Enable Branch coverage for SwitchStmt (PR #113112)

NAKAMURA Takumi via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Sun Oct 27 10:52:15 PDT 2024


https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/113112

>From ec05cc37e1177f06c9a44a1e39dadc9306cc5c68 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Mon, 21 Oct 2024 08:09:31 +0900
Subject: [PATCH 1/5] [Coverage][Single] Enable Branch coverage for SwitchStmt

---
 clang/lib/CodeGen/CGStmt.cpp                  | 12 ++++++++++
 clang/lib/CodeGen/CoverageMappingGen.cpp      | 22 ++++++++++---------
 .../CoverageMapping/single-byte-counters.cpp  | 13 ++++++-----
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index dbc1ce9bf993cd..80fe5cf183de16 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -2259,6 +2259,18 @@ void CodeGenFunction::EmitSwitchStmt(const SwitchStmt &S) {
 
   ConditionScope.ForceCleanup();
 
+  // Close the last case (or DefaultBlock).
+  EmitBranch(SwitchExit.getBlock());
+
+  // Insert a False Counter if SwitchStmt doesn't have DefaultStmt.
+  if (getIsCounterPair(S.getCond()).second) {
+    auto *ImplicitDefaultBlock = createBasicBlock("sw.false");
+    EmitBlock(ImplicitDefaultBlock);
+    incrementProfileCounter(true, S.getCond());
+    Builder.CreateBr(SwitchInsn->getDefaultDest());
+    SwitchInsn->setDefaultDest(ImplicitDefaultBlock);
+  }
+
   // Emit continuation.
   EmitBlock(SwitchExit.getBlock(), true);
   incrementProfileCounter(&S);
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index a331d5bc68286b..c5fdf23299e4e9 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -958,6 +958,14 @@ struct CounterCoverageMappingBuilder
     return {ExecCnt, SkipCnt};
   }
 
+  Counter getSwitchImplicitDefaultCounter(const Stmt *Cond, Counter ParentCount,
+                                          Counter CaseCountSum) {
+    return (
+        llvm::EnableSingleByteCoverage
+            ? Counter::getCounter(CounterMap[Cond].second = NextCounterNum++)
+            : subtractCounters(ParentCount, CaseCountSum));
+  }
+
   bool IsCounterEqual(Counter OutCount, Counter ParentCount) {
     if (OutCount == ParentCount)
       return true;
@@ -1885,7 +1893,7 @@ struct CounterCoverageMappingBuilder
       propagateCounts(Counter::getZero(), Body);
     BreakContinue BC = BreakContinueStack.pop_back_val();
 
-    if (!BreakContinueStack.empty() && !llvm::EnableSingleByteCoverage)
+    if (!BreakContinueStack.empty())
       BreakContinueStack.back().ContinueCount = addCounters(
           BreakContinueStack.back().ContinueCount, BC.ContinueCount);
 
@@ -1900,11 +1908,6 @@ struct CounterCoverageMappingBuilder
     MostRecentLocation = getStart(S);
     handleFileExit(ExitLoc);
 
-    // When single byte coverage mode is enabled, do not create branch region by
-    // early returning.
-    if (llvm::EnableSingleByteCoverage)
-      return;
-
     // Create a Branch Region around each Case. Subtract the case's
     // counter from the Parent counter to track the "False" branch count.
     Counter CaseCountSum;
@@ -1920,7 +1923,8 @@ struct CounterCoverageMappingBuilder
     // the hidden branch, which will be added later by the CodeGen. This region
     // will be associated with the switch statement's condition.
     if (!HasDefaultCase) {
-      Counter DefaultCount = subtractCounters(ParentCount, CaseCountSum);
+      Counter DefaultCount = getSwitchImplicitDefaultCounter(
+          S->getCond(), ParentCount, CaseCountSum);
       createBranchRegion(S->getCond(), Counter::getZero(), DefaultCount);
     }
   }
@@ -1929,9 +1933,7 @@ struct CounterCoverageMappingBuilder
     extendRegion(S);
 
     SourceMappingRegion &Parent = getRegion();
-    Counter Count = llvm::EnableSingleByteCoverage
-                        ? getRegionCounter(S)
-                        : addCounters(Parent.getCounter(), getRegionCounter(S));
+    Counter Count = addCounters(Parent.getCounter(), getRegionCounter(S));
 
     // Reuse the existing region if it starts at our label. This is typical of
     // the first case in a switch.
diff --git a/clang/test/CoverageMapping/single-byte-counters.cpp b/clang/test/CoverageMapping/single-byte-counters.cpp
index d20b695bc2636a..464fa370d86f09 100644
--- a/clang/test/CoverageMapping/single-byte-counters.cpp
+++ b/clang/test/CoverageMapping/single-byte-counters.cpp
@@ -34,19 +34,22 @@ int testIfElseReturn(int x) { // CHECK-NEXT: File 0, [[@LINE]]:29 -> [[@LINE+9]]
 }
 
 // CHECK-NEXT: testSwitch
-int testSwitch(int x) { // CHECK-NEXT: File 0, [[@LINE]]:23 -> [[@LINE+17]]:2 = [[C30:#0]]
+int testSwitch(int x) { // CHECK-NEXT: File 0, [[@LINE]]:23 -> [[@LINE+20]]:2 = [[C30:#0]]
   int result;
   switch (x) {
-                        // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+10]]:15 = 0
-  case 1:               // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = [[C31:#2]]
+                        // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+13]]:15 = 0
+  case 1:               // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = [[C31:#2]]
+                        // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = [[C31]], 0
     result = 1;
     break;
                         // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:11 -> [[@LINE+1]]:3 = 0
-  case 2:               // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = [[C32:#3]]
+  case 2:               // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = [[C32:#3]]
+                        // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:9 = [[C32]], 0
     result = 2;
     break;
                         // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:11 -> [[@LINE+1]]:3 = 0
-  default:              // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:15 = [[C3D:#4]]
+  default:              // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:15 = [[C3D:#4]]
+                        // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:3 -> [[@LINE-1]]:10 = [[C3D]], 0
     result = 0;
   }
                         // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE+1]]:3 = [[C3E:#1]]

>From 03cfce188cb45adbe78f7e77c2fdd244650f5a3c Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Wed, 23 Oct 2024 14:39:35 +0000
Subject: [PATCH 2/5] CGF::markStmtAsUsed

---
 clang/lib/CodeGen/CodeGenFunction.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 89ac3b342d0a7c..fcad1cbcae5e3d 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1624,6 +1624,9 @@ class CodeGenFunction : public CodeGenTypeCache {
     return PGO.getIsCounterPair(S);
   }
 
+  void markStmtAsUsed(bool Skipped, const Stmt *S) {
+    PGO.markStmtAsUsed(Skipped, S);
+  }
   void markStmtMaybeUsed(const Stmt *S) { PGO.markStmtMaybeUsed(S); }
 
   /// Increment the profiler's counter for the given statement by \p StepV.

>From afc8481f7cf20da7de4e95a60bf3ccdb04bd08f3 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Wed, 23 Oct 2024 14:39:35 +0000
Subject: [PATCH 3/5] CGF.markStmtMaybeUsed for binop

---
 clang/lib/CodeGen/CGExprScalar.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 74e93f889f4261..9e8533ca90a5cd 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -4970,7 +4970,8 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) {
         CGF.incrementProfileCounter(E->getRHS());
         CGF.EmitBranch(FBlock);
         CGF.EmitBlock(FBlock);
-      }
+      } else
+        CGF.markStmtMaybeUsed(E->getRHS());
 
       CGF.MCDCLogOpStack.pop_back();
       // If the top of the logical operator nest, update the MCDC bitmap.
@@ -5112,7 +5113,8 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) {
         CGF.incrementProfileCounter(E->getRHS());
         CGF.EmitBranch(FBlock);
         CGF.EmitBlock(FBlock);
-      }
+      } else
+        CGF.markStmtMaybeUsed(E->getRHS());
 
       CGF.MCDCLogOpStack.pop_back();
       // If the top of the logical operator nest, update the MCDC bitmap.

>From ab84f17fc181cb4b38693dc6ea80ac44f44bf990 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sun, 27 Oct 2024 20:28:26 +0900
Subject: [PATCH 4/5] Introduce skeleton getSwitchImplicitDefaultCounter()

---
 clang/lib/CodeGen/CoverageMappingGen.cpp | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 1782434bdb9aa6..532f6e8ba18201 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -947,6 +947,11 @@ struct CounterCoverageMappingBuilder
     return {ExecCnt, Builder.subtract(ParentCnt, ExecCnt)};
   }
 
+  Counter getSwitchImplicitDefaultCounter(const Stmt *Cond, Counter ParentCount,
+                                          Counter CaseCountSum) {
+    return Builder.subtract(ParentCount, CaseCountSum);
+  }
+
   bool IsCounterEqual(Counter OutCount, Counter ParentCount) {
     if (OutCount == ParentCount)
       return true;
@@ -1903,7 +1908,8 @@ struct CounterCoverageMappingBuilder
     // the hidden branch, which will be added later by the CodeGen. This region
     // will be associated with the switch statement's condition.
     if (!HasDefaultCase) {
-      Counter DefaultCount = subtractCounters(ParentCount, CaseCountSum);
+      Counter DefaultCount = getSwitchImplicitDefaultCounter(
+          S->getCond(), ParentCount, CaseCountSum);
       createBranchRegion(S->getCond(), Counter::getZero(), DefaultCount);
     }
   }

>From a4608854e1b727817db3cc01234f97e78285f8c7 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sun, 27 Oct 2024 20:40:44 +0900
Subject: [PATCH 5/5] Update getSwitchImplicitDefaultCounter

---
 clang/lib/CodeGen/CoverageMappingGen.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index d4a1b2613eab92..cb861e61d32727 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -960,7 +960,10 @@ struct CounterCoverageMappingBuilder
 
   Counter getSwitchImplicitDefaultCounter(const Stmt *Cond, Counter ParentCount,
                                           Counter CaseCountSum) {
-    return Builder.subtract(ParentCount, CaseCountSum);
+    return (
+        llvm::EnableSingleByteCoverage
+            ? Counter::getCounter(CounterMap[Cond].second = NextCounterNum++)
+            : Builder.subtract(ParentCount, CaseCountSum));
   }
 
   bool IsCounterEqual(Counter OutCount, Counter ParentCount) {



More information about the llvm-branch-commits mailing list