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 <geek4ci...@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 <geek4ci...@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 <geek4ci...@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 <geek4ci...@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; -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits