https://github.com/chapuni updated https://github.com/llvm/llvm-project/pull/125405
>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/3] [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 f70a6c8686617963c55854c2d8c6fa8607ca0806 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Sun, 2 Feb 2025 22:10:25 +0900 Subject: [PATCH 2/3] [MC/DC] Handle __builtin_expect as if parenthses Fixes #124565 --- clang/include/clang/AST/IgnoreExpr.h | 9 +++++++++ clang/lib/CodeGen/CodeGenFunction.cpp | 13 ++++++++----- clang/lib/CodeGen/CodeGenPGO.cpp | 8 +++++--- clang/test/CoverageMapping/mcdc-nested-expr.cpp | 5 ++++- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/AST/IgnoreExpr.h b/clang/include/clang/AST/IgnoreExpr.h index 917bada61fa6f..c366aa1766748 100644 --- a/clang/include/clang/AST/IgnoreExpr.h +++ b/clang/include/clang/AST/IgnoreExpr.h @@ -134,6 +134,15 @@ inline Expr *IgnoreElidableImplicitConstructorSingleStep(Expr *E) { return E; } +inline Expr *IgnoreBuiltinExpectSingleStep(Expr *E) { + if (auto *CE = dyn_cast<CallExpr>(E)) { + if (const FunctionDecl *FD = CE->getDirectCallee()) + if (FD->getBuiltinID() == Builtin::BI__builtin_expect) + return CE->getArg(0); + } + return E; +} + inline Expr *IgnoreImplicitAsWrittenSingleStep(Expr *E) { if (auto *ICE = dyn_cast<ImplicitCastExpr>(E)) return ICE->getSubExprAsWritten(); diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index bbef277a52448..05766aa7693d2 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -27,6 +27,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/Expr.h" +#include "clang/AST/IgnoreExpr.h" #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtObjC.h" #include "clang/Basic/Builtins.h" @@ -1748,12 +1749,14 @@ bool CodeGenFunction::ConstantFoldsToSimpleInteger(const Expr *Cond, /// Strip parentheses and simplistic logical-NOT operators. const Expr *CodeGenFunction::stripCond(const Expr *C) { - while (const UnaryOperator *Op = dyn_cast<UnaryOperator>(C->IgnoreParens())) { - if (Op->getOpcode() != UO_LNot) - break; - C = Op->getSubExpr(); + while (true) { + const Expr *SC = IgnoreExprNodes(C, IgnoreParensSingleStep, + IgnoreBuiltinExpectSingleStep, + IgnoreImplicitCastsSingleStep); + if (C == SC) + return SC; + C = SC; } - return C->IgnoreParens(); } /// Determine whether the given condition is an instrumentable condition diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 792373839107f..0fd49b880bba3 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -247,8 +247,9 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { } if (const Expr *E = dyn_cast<Expr>(S)) { - const BinaryOperator *BinOp = dyn_cast<BinaryOperator>(E->IgnoreParens()); - if (BinOp && BinOp->isLogicalOp()) { + if (const auto *BinOp = + dyn_cast<BinaryOperator>(CodeGenFunction::stripCond(E)); + BinOp && BinOp->isLogicalOp()) { /// Check for "split-nested" logical operators. This happens when a new /// boolean expression logical-op nest is encountered within an existing /// boolean expression, separated by a non-logical operator. For @@ -280,7 +281,8 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> { return true; if (const Expr *E = dyn_cast<Expr>(S)) { - const BinaryOperator *BinOp = dyn_cast<BinaryOperator>(E->IgnoreParens()); + const BinaryOperator *BinOp = + dyn_cast<BinaryOperator>(CodeGenFunction::stripCond(E)); if (BinOp && BinOp->isLogicalOp()) { assert(LogOpStack.back() == BinOp); LogOpStack.pop_back(); diff --git a/clang/test/CoverageMapping/mcdc-nested-expr.cpp b/clang/test/CoverageMapping/mcdc-nested-expr.cpp index bb82873e3b600..0614a2b7ab8c1 100644 --- a/clang/test/CoverageMapping/mcdc-nested-expr.cpp +++ b/clang/test/CoverageMapping/mcdc-nested-expr.cpp @@ -21,8 +21,11 @@ bool func_condop(bool a, bool b, bool c) { // 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); + // CHECK: Decision,File 0, [[@LINE-1]]:10 -> [[#L:@LINE-1]]:45 = M:4, C:3 + // CHECK: Branch,File 0, [[#L]]:10 -> [[#L]]:11 = (#0 - #1), #1 [1,0,2] + // CHECK: Branch,File 0, [[#L]]:32 -> [[#L]]:33 = #2, (#1 - #2) [2,3,0] + // CHECK: Branch,File 0, [[#L]]:37 -> [[#L]]:38 = #3, (#2 - #3) [3,0,0] } // LNot among BinOp(s) >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 3/3] 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