[clang] [MC/DC] Refactor MCDC::State::Decision. NFC. (PR #125408)
NAKAMURA Takumi via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 28 23:08:18 PST 2025
https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/125408
>From a6d4be0e4b05b411c7160385485cfed0f173965c Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sun, 2 Feb 2025 21:55:43 +0900
Subject: [PATCH 1/4] [MC/DC] Update CoverageMapping tests
To resolve the error, rename mcdc-error-nests.cpp -> mcdc-nested-expr.cpp at first.
- `func_condop`
A corner case that contains close decisions.
- `func_expect`
Uses `__builtin_expect`. (#124565)
- `func_lnot`
Contains logical not(s) `!` among MC/DC binary operators. (#124563)
mcdc-single-cond.cpp is for #95336.
---
.../test/CoverageMapping/mcdc-error-nests.cpp | 10 --
.../test/CoverageMapping/mcdc-nested-expr.cpp | 34 +++++++
.../test/CoverageMapping/mcdc-single-cond.cpp | 97 +++++++++++++++++++
3 files changed, 131 insertions(+), 10 deletions(-)
delete mode 100644 clang/test/CoverageMapping/mcdc-error-nests.cpp
create mode 100644 clang/test/CoverageMapping/mcdc-nested-expr.cpp
create mode 100644 clang/test/CoverageMapping/mcdc-single-cond.cpp
diff --git a/clang/test/CoverageMapping/mcdc-error-nests.cpp b/clang/test/CoverageMapping/mcdc-error-nests.cpp
deleted file mode 100644
index 3add2b9ccd3fb..0000000000000
--- a/clang/test/CoverageMapping/mcdc-error-nests.cpp
+++ /dev/null
@@ -1,10 +0,0 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s 2>&1| FileCheck %s
-
-// "Split-nest" -- boolean expressions within boolean expressions.
-extern bool bar(bool);
-bool func_split_nest(bool a, bool b, bool c, bool d, bool e, bool f, bool g) {
- bool res = a && b && c && bar(d && e) && f && g;
- return bar(res);
-}
-
-// CHECK: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression.
diff --git a/clang/test/CoverageMapping/mcdc-nested-expr.cpp b/clang/test/CoverageMapping/mcdc-nested-expr.cpp
new file mode 100644
index 0000000000000..bb82873e3b600
--- /dev/null
+++ b/clang/test/CoverageMapping/mcdc-nested-expr.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s 2> %t.stderr.txt | FileCheck %s
+// RUN: FileCheck %s --check-prefix=WARN < %t.stderr.txt
+
+// "Split-nest" -- boolean expressions within boolean expressions.
+extern bool bar(bool);
+// CHECK: func_split_nest{{.*}}:
+bool func_split_nest(bool a, bool b, bool c, bool d, bool e, bool f, bool g) {
+ // WARN: :[[@LINE+1]]:14: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression.
+ bool res = a && b && c && bar(d && e) && f && g;
+ return bar(res);
+}
+
+// The inner expr begins with the same Loc as the outer expr
+// CHECK: func_condop{{.*}}:
+bool func_condop(bool a, bool b, bool c) {
+ // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression.
+ return (a && b ? true : false) && c;
+}
+
+// __builtin_expect
+// Treated as parentheses.
+// CHECK: func_expect{{.*}}:
+bool func_expect(bool a, bool b, bool c) {
+ // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression.
+ return a || __builtin_expect(b && c, true);
+}
+
+// LNot among BinOp(s)
+// Doesn't split exprs.
+// CHECK: func_lnot{{.*}}:
+bool func_lnot(bool a, bool b, bool c, bool d) {
+ // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression.
+ return !(a || b) && !(c && d);
+}
diff --git a/clang/test/CoverageMapping/mcdc-single-cond.cpp b/clang/test/CoverageMapping/mcdc-single-cond.cpp
new file mode 100644
index 0000000000000..b1eeea879e521
--- /dev/null
+++ b/clang/test/CoverageMapping/mcdc-single-cond.cpp
@@ -0,0 +1,97 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -disable-llvm-passes -emit-llvm -o %t2.ll %s | FileCheck %s --check-prefixes=MM,MM2
+// RUN: FileCheck %s --check-prefixes=LL,LL2 < %t2.ll
+
+// LL: define{{.+}}func_cond{{.+}}(
+// MM: func_cond{{.*}}:
+int func_cond(bool a, bool b) {
+ // %mcdc.addr* are emitted by static order.
+ // LL: %[[MA:mcdc.addr.*]] = alloca i32, align 4
+ // LL: call void @llvm.instrprof.mcdc.parameters(ptr @[[PROFN:.+]], i64 [[#H:]], i32 [[#BS:]])
+ int count = 0;
+ if (a)
+ // NB=2 Single cond
+ // MM2-NOT: Decision
+ ++count;
+ if (a ? true : false)
+ // NB=2,2 Wider decision comes first.
+ // MA2 has C:2
+ // MA3 has C:1
+ ++count;
+ if (a && b ? true : false)
+ // NB=2,3 Wider decision comes first.
+ // MM2: Decision,File 0, [[@LINE-2]]:7 -> [[#L:@LINE-2]]:13 = M:[[#I:3]], C:2
+ // MM: Branch,File 0, [[#L]]:7 -> [[#L]]:8 = #6, (#0 - #6) [1,2,0]
+ // MM: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = #7, (#6 - #7) [2,0,0]
+ // LL: store i32 0, ptr %[[MA]], align 4
+ // LL: = load i32, ptr %[[MA]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
+ // LL: = load i32, ptr %[[MA]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
+ // LL2: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64 [[#H]], i32 [[#B:0]], ptr %[[MA]])
+ ++count;
+ while (a || true) {
+ // NB=3 BinOp only
+ // MM: Decision,File 0, [[@LINE-2]]:10 -> [[#L:@LINE-2]]:19 = M:[[#I:I+3]], C:2
+ // MM: Branch,File 0, [[#L]]:10 -> [[#L]]:11 = (#0 - #9), #9 [1,0,2]
+ // MM: Branch,File 0, [[#L]]:15 -> [[#L]]:19 = (#9 - #10), 0 [2,0,0]
+ // LL: store i32 0, ptr %[[MA]], align 4
+ // LL: = load i32, ptr %[[MA]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
+ // LL: = load i32, ptr %[[MA]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
+ // LL2: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64 [[#H]], i32 [[#B:B+3]], ptr %[[MA]])
+ ++count;
+ break;
+ }
+ while (a || true ? false : true) {
+ // Wider decision comes first.
+ // MM2: Decision,File 0, [[@LINE-2]]:10 -> [[#L:@LINE-2]]:19 = M:[[#I:I+3]], C:2
+ // MM: Branch,File 0, [[#L]]:10 -> [[#L]]:11 = ((#0 + #11) - #13), #13 [1,0,2]
+ // MM: Branch,File 0, [[#L]]:15 -> [[#L]]:19 = (#13 - #14), 0 [2,0,0]
+ // LL: store i32 0, ptr %[[MA]], align 4
+ // LL: = load i32, ptr %[[MA]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
+ // LL: = load i32, ptr %[[MA]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
+ // LL: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64 [[#H]], i32 [[#B:B+3]], ptr %[[MA]])
+ ++count;
+ }
+ do {
+ ++count;
+ } while (a && false);
+ // BinOp only
+ // MM: Decision,File 0, [[@LINE-2]]:12 -> [[#L:@LINE-2]]:22 = M:[[#I:I+3]], C:2
+ // MM: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = #16, ((#0 + #15) - #16) [1,2,0]
+ // MM: Branch,File 0, [[#L]]:17 -> [[#L]]:22 = 0, (#16 - #17) [2,0,0]
+ // LL: store i32 0, ptr %[[MA]], align 4
+ // LL: = load i32, ptr %[[MA]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
+ // LL: = load i32, ptr %[[MA]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
+ // LL2: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64 [[#H]], i32 [[#B:B+3]], ptr %[[MA]])
+ do {
+ ++count;
+ } while (a && false ? true : false);
+ // Wider decision comes first.
+ // MM2: Decision,File 0, [[@LINE-2]]:12 -> [[#L:@LINE-2]]:22 = M:15, C:2
+ // MM: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = #20, ((#0 + #18) - #20) [1,2,0]
+ // MM: Branch,File 0, [[#L]]:17 -> [[#L]]:22 = 0, (#20 - #21) [2,0,0]
+ // LL: store i32 0, ptr %[[MA]], align 4
+ // LL: = load i32, ptr %[[MA]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
+ // LL: = load i32, ptr %[[MA]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
+ // LL: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64 [[#H]], i32 [[#B:B+3]], ptr %[[MA]])
+ // FIXME: Confirm (B+3==BS)
+ for (int i = 0; i < (a ? 2 : 1); ++i) {
+ // Simple nested decision (different column)
+ // MM2-NOT: Decision
+ // LL2-NOT: call void @llvm.instrprof.mcdc.tvbitmap.update
+ ++count;
+ }
+ for (int i = 0; i >= 4 ? false : true; ++i) {
+ // Wider decision comes first.
+ ++count;
+ }
+ return count;
+}
>From d414f29ed8732c77fdcd05cc3b066e9ee0d9de07 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sun, 2 Feb 2025 21:59:33 +0900
Subject: [PATCH 2/4] [MC/DC] Refactor MCDC::State::Decision. NFC.
Introduce `ID` and `InvalidID`. Then `DecisionByStmt` can have three
states.
* Not assigned if the Stmt(Expr) doesn't exist.
* When `DecisionByStmt[Expr]` exists:
* Invalid and should be ignored if `ID == Invalid`.
* Valid if `ID != Invalid`. Other member will be filled in the
Mapper.
---
clang/lib/CodeGen/CodeGenPGO.cpp | 2 +-
clang/lib/CodeGen/CoverageMappingGen.cpp | 6 ++----
clang/lib/CodeGen/MCDCState.h | 16 +++++++++++++++-
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 792373839107f..0331ff83e633f 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -310,7 +310,7 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
}
// Otherwise, allocate the Decision.
- MCDCState.DecisionByStmt[BinOp].BitmapIdx = 0;
+ MCDCState.DecisionByStmt[BinOp].ID = MCDCState.DecisionByStmt.size();
}
return true;
}
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index f09157771d2b5..4dbc0c70e34d6 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -2197,10 +2197,8 @@ struct CounterCoverageMappingBuilder
// Update the state for CodeGenPGO
assert(MCDCState.DecisionByStmt.contains(E));
- MCDCState.DecisionByStmt[E] = {
- MCDCState.BitmapBits, // Top
- std::move(Builder.Indices),
- };
+ MCDCState.DecisionByStmt[E].update(MCDCState.BitmapBits, // Top
+ std::move(Builder.Indices));
auto DecisionParams = mcdc::DecisionParameters{
MCDCState.BitmapBits += NumTVs, // Tail
diff --git a/clang/lib/CodeGen/MCDCState.h b/clang/lib/CodeGen/MCDCState.h
index e0dd28ff90ed1..0b6f5f28235f4 100644
--- a/clang/lib/CodeGen/MCDCState.h
+++ b/clang/lib/CodeGen/MCDCState.h
@@ -16,6 +16,8 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ProfileData/Coverage/MCDCTypes.h"
+#include <cassert>
+#include <limits>
namespace clang {
class Stmt;
@@ -30,8 +32,20 @@ struct State {
unsigned BitmapBits = 0;
struct Decision {
+ using IndicesTy = llvm::SmallVector<std::array<int, 2>>;
+ static constexpr auto InvalidID = std::numeric_limits<unsigned>::max();
+
unsigned BitmapIdx;
- llvm::SmallVector<std::array<int, 2>> Indices;
+ IndicesTy Indices;
+ unsigned ID = InvalidID;
+
+ bool isValid() const { return ID != InvalidID; }
+
+ void update(unsigned I, IndicesTy &&X) {
+ assert(ID != InvalidID);
+ BitmapIdx = I;
+ Indices = std::move(X);
+ }
};
llvm::DenseMap<const Stmt *, Decision> DecisionByStmt;
>From 079bf69ee66b3cda8206f790185a8bb325ec7064 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sat, 1 Mar 2025 15:37:35 +0900
Subject: [PATCH 3/4] isValid()
---
clang/lib/CodeGen/MCDCState.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/MCDCState.h b/clang/lib/CodeGen/MCDCState.h
index 0b6f5f28235f4..4b89ddd12d672 100644
--- a/clang/lib/CodeGen/MCDCState.h
+++ b/clang/lib/CodeGen/MCDCState.h
@@ -42,7 +42,7 @@ struct State {
bool isValid() const { return ID != InvalidID; }
void update(unsigned I, IndicesTy &&X) {
- assert(ID != InvalidID);
+ assert(isValid());
BitmapIdx = I;
Indices = std::move(X);
}
>From 8b35e76018417c301dc757ea5446292dd3a3996c Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Sat, 1 Mar 2025 15:52:53 +0900
Subject: [PATCH 4/4] Remove mcdc-single-cond.cpp
---
.../test/CoverageMapping/mcdc-single-cond.cpp | 97 -------------------
1 file changed, 97 deletions(-)
delete mode 100644 clang/test/CoverageMapping/mcdc-single-cond.cpp
diff --git a/clang/test/CoverageMapping/mcdc-single-cond.cpp b/clang/test/CoverageMapping/mcdc-single-cond.cpp
deleted file mode 100644
index b1eeea879e521..0000000000000
--- a/clang/test/CoverageMapping/mcdc-single-cond.cpp
+++ /dev/null
@@ -1,97 +0,0 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -disable-llvm-passes -emit-llvm -o %t2.ll %s | FileCheck %s --check-prefixes=MM,MM2
-// RUN: FileCheck %s --check-prefixes=LL,LL2 < %t2.ll
-
-// LL: define{{.+}}func_cond{{.+}}(
-// MM: func_cond{{.*}}:
-int func_cond(bool a, bool b) {
- // %mcdc.addr* are emitted by static order.
- // LL: %[[MA:mcdc.addr.*]] = alloca i32, align 4
- // LL: call void @llvm.instrprof.mcdc.parameters(ptr @[[PROFN:.+]], i64 [[#H:]], i32 [[#BS:]])
- int count = 0;
- if (a)
- // NB=2 Single cond
- // MM2-NOT: Decision
- ++count;
- if (a ? true : false)
- // NB=2,2 Wider decision comes first.
- // MA2 has C:2
- // MA3 has C:1
- ++count;
- if (a && b ? true : false)
- // NB=2,3 Wider decision comes first.
- // MM2: Decision,File 0, [[@LINE-2]]:7 -> [[#L:@LINE-2]]:13 = M:[[#I:3]], C:2
- // MM: Branch,File 0, [[#L]]:7 -> [[#L]]:8 = #6, (#0 - #6) [1,2,0]
- // MM: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = #7, (#6 - #7) [2,0,0]
- // LL: store i32 0, ptr %[[MA]], align 4
- // LL: = load i32, ptr %[[MA]], align 4
- // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
- // LL: = load i32, ptr %[[MA]], align 4
- // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
- // LL2: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64 [[#H]], i32 [[#B:0]], ptr %[[MA]])
- ++count;
- while (a || true) {
- // NB=3 BinOp only
- // MM: Decision,File 0, [[@LINE-2]]:10 -> [[#L:@LINE-2]]:19 = M:[[#I:I+3]], C:2
- // MM: Branch,File 0, [[#L]]:10 -> [[#L]]:11 = (#0 - #9), #9 [1,0,2]
- // MM: Branch,File 0, [[#L]]:15 -> [[#L]]:19 = (#9 - #10), 0 [2,0,0]
- // LL: store i32 0, ptr %[[MA]], align 4
- // LL: = load i32, ptr %[[MA]], align 4
- // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
- // LL: = load i32, ptr %[[MA]], align 4
- // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
- // LL2: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64 [[#H]], i32 [[#B:B+3]], ptr %[[MA]])
- ++count;
- break;
- }
- while (a || true ? false : true) {
- // Wider decision comes first.
- // MM2: Decision,File 0, [[@LINE-2]]:10 -> [[#L:@LINE-2]]:19 = M:[[#I:I+3]], C:2
- // MM: Branch,File 0, [[#L]]:10 -> [[#L]]:11 = ((#0 + #11) - #13), #13 [1,0,2]
- // MM: Branch,File 0, [[#L]]:15 -> [[#L]]:19 = (#13 - #14), 0 [2,0,0]
- // LL: store i32 0, ptr %[[MA]], align 4
- // LL: = load i32, ptr %[[MA]], align 4
- // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
- // LL: = load i32, ptr %[[MA]], align 4
- // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
- // LL: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64 [[#H]], i32 [[#B:B+3]], ptr %[[MA]])
- ++count;
- }
- do {
- ++count;
- } while (a && false);
- // BinOp only
- // MM: Decision,File 0, [[@LINE-2]]:12 -> [[#L:@LINE-2]]:22 = M:[[#I:I+3]], C:2
- // MM: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = #16, ((#0 + #15) - #16) [1,2,0]
- // MM: Branch,File 0, [[#L]]:17 -> [[#L]]:22 = 0, (#16 - #17) [2,0,0]
- // LL: store i32 0, ptr %[[MA]], align 4
- // LL: = load i32, ptr %[[MA]], align 4
- // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
- // LL: = load i32, ptr %[[MA]], align 4
- // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
- // LL2: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64 [[#H]], i32 [[#B:B+3]], ptr %[[MA]])
- do {
- ++count;
- } while (a && false ? true : false);
- // Wider decision comes first.
- // MM2: Decision,File 0, [[@LINE-2]]:12 -> [[#L:@LINE-2]]:22 = M:15, C:2
- // MM: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = #20, ((#0 + #18) - #20) [1,2,0]
- // MM: Branch,File 0, [[#L]]:17 -> [[#L]]:22 = 0, (#20 - #21) [2,0,0]
- // LL: store i32 0, ptr %[[MA]], align 4
- // LL: = load i32, ptr %[[MA]], align 4
- // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
- // LL: = load i32, ptr %[[MA]], align 4
- // LL: store i32 %{{.+}}, ptr %[[MA]], align 4
- // LL: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64 [[#H]], i32 [[#B:B+3]], ptr %[[MA]])
- // FIXME: Confirm (B+3==BS)
- for (int i = 0; i < (a ? 2 : 1); ++i) {
- // Simple nested decision (different column)
- // MM2-NOT: Decision
- // LL2-NOT: call void @llvm.instrprof.mcdc.tvbitmap.update
- ++count;
- }
- for (int i = 0; i >= 4 ? false : true; ++i) {
- // Wider decision comes first.
- ++count;
- }
- return count;
-}
More information about the cfe-commits
mailing list