llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: NAKAMURA Takumi (chapuni) <details> <summary>Changes</summary> In the current implementation, `!(a || b) && c` was not treated as one Decision with three terms. Fixes #<!-- -->124563 --- Full diff: https://github.com/llvm/llvm-project/pull/125406.diff 6 Files Affected: - (modified) clang/include/clang/AST/IgnoreExpr.h (+8) - (modified) clang/lib/CodeGen/CodeGenFunction.cpp (+7-5) - (modified) clang/lib/CodeGen/CodeGenPGO.cpp (+5-3) - (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+12) - (modified) clang/test/CoverageMapping/mcdc-nested-expr.cpp (+5-1) - (modified) clang/test/Profile/c-mcdc-not.c (+95) ``````````diff diff --git a/clang/include/clang/AST/IgnoreExpr.h b/clang/include/clang/AST/IgnoreExpr.h index 917bada61fa6fdd..c48c0c0daf81517 100644 --- a/clang/include/clang/AST/IgnoreExpr.h +++ b/clang/include/clang/AST/IgnoreExpr.h @@ -134,6 +134,14 @@ inline Expr *IgnoreElidableImplicitConstructorSingleStep(Expr *E) { return E; } +inline Expr *IgnoreUOpLNotSingleStep(Expr *E) { + if (auto *UO = dyn_cast<UnaryOperator>(E)) { + if (UO->getOpcode() == UO_LNot) + return UO->getSubExpr(); + } + 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 bbef277a524480b..2c380ac926b1e70 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,13 @@ 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, IgnoreUOpLNotSingleStep); + 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 792373839107f0a..0fd49b880bba305 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/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index f09157771d2b5c0..9bf73cf27a5fa9a 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -799,6 +799,12 @@ struct MCDCCoverageBuilder { /// Return the LHS Decision ([0,0] if not set). const mcdc::ConditionIDs &back() const { return DecisionStack.back(); } + void swapConds() { + if (DecisionStack.empty()) + return; + + std::swap(DecisionStack.back()[false], DecisionStack.back()[true]); + } /// Push the binary operator statement to track the nest level and assign IDs /// to the operator's LHS and RHS. The RHS may be a larger subtree that is /// broken up on successive levels. @@ -2241,6 +2247,12 @@ struct CounterCoverageMappingBuilder SM.isInSystemHeader(SM.getSpellingLoc(E->getEndLoc()))); } + void VisitUnaryLNot(const UnaryOperator *E) { + MCDCBuilder.swapConds(); + Visit(E->getSubExpr()); + MCDCBuilder.swapConds(); + } + void VisitBinLAnd(const BinaryOperator *E) { if (isExprInSystemHeader(E)) { LeafExprSet.insert(E); diff --git a/clang/test/CoverageMapping/mcdc-nested-expr.cpp b/clang/test/CoverageMapping/mcdc-nested-expr.cpp index bb82873e3b600d0..a49dad0b336126f 100644 --- a/clang/test/CoverageMapping/mcdc-nested-expr.cpp +++ b/clang/test/CoverageMapping/mcdc-nested-expr.cpp @@ -29,6 +29,10 @@ bool func_expect(bool a, bool b, bool c) { // 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); + // CHECK: Decision,File 0, [[@LINE-1]]:10 -> [[#L:@LINE-1]]:32 = M:5, C:4 + // CHECK: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = (#0 - #2), #2 [1,0,3] + // CHECK: Branch,File 0, [[#L]]:17 -> [[#L]]:18 = (#2 - #3), #3 [3,0,2] + // CHECK: Branch,File 0, [[#L]]:25 -> [[#L]]:26 = #4, (#1 - #4) [2,4,0] + // CHECK: Branch,File 0, [[#L]]:30 -> [[#L]]:31 = #5, (#4 - #5) [4,0,0] } diff --git a/clang/test/Profile/c-mcdc-not.c b/clang/test/Profile/c-mcdc-not.c index 7083aa226fb9520..d6d79bf15cb086e 100644 --- a/clang/test/Profile/c-mcdc-not.c +++ b/clang/test/Profile/c-mcdc-not.c @@ -85,3 +85,98 @@ int test(int a, int b, int c, int d, int e, int f) { // MCDC: %[[BITS:.+]] = load i8, ptr %[[LAB4]], align 1 // MCDC: %[[LAB8:[0-9]+]] = or i8 %[[BITS]], %[[LAB7]] // MCDC: store i8 %[[LAB8]], ptr %[[LAB4]], align 1 + +int internot(int a, int b, int c, int d, int e, int f) { + return !(!(!a && b) || !(!!(!c && d) || !(e && !f))); +} + +// MCDC-LABEL: @internot( +// MCDC-DAG: store i32 0, ptr %mcdc.addr, align 4 + +// Branch #2, (#0 - #2) [1,3,0] +// !a [+0 => b][+6 => END] +// MCDC-DAG: %[[A:.+]] = load i32, ptr %a.addr, align 4 +// MCDC-DAG: %[[AB:.+]] = icmp ne i32 %[[A]], 0 +// MCDC-DAG: %[[AN:.+]] = xor i1 %[[AB]], true +// MCDC-DAG: %[[M1:.+]] = load i32, ptr %mcdc.addr, align 4 +// MCDC-DAG: %[[M1T:.+]] = add i32 %[[M1]], 0 +// MCDC-DAG: %[[M1F:.+]] = add i32 %[[M1]], 6 +// MCDC-DAG: %[[M1S:.+]] = select i1 %[[AN]], i32 %[[M1T]], i32 %[[M1F]] +// MCDC-DAG: store i32 %[[M1S]], ptr %mcdc.addr, align 4 + +// Branch #3, (#2 - #3) [3,2,0] +// b [+0 => c][+7 => END] +// MCDC-DAG: %[[B:.+]] = load i32, ptr %b.addr, align 4 +// MCDC-DAG: %[[BB:.+]] = icmp ne i32 %[[B]], 0 +// MCDC-DAG: %[[M3:.+]] = load i32, ptr %mcdc.addr, align 4 +// MCDC-DAG: %[[M3T:.+]] = add i32 %[[M3]], 0 +// MCDC-DAG: %[[M3F:.+]] = add i32 %[[M3]], 7 +// MCDC-DAG: %[[M3S:.+]] = select i1 %[[BB]], i32 %[[M3T]], i32 %[[M3F]] +// MCDC-DAG: store i32 %[[M3S]], ptr %mcdc.addr, align 4 + +// Branch #5, (#1 - #5) [2,5,4] +// !c [+0 => d][+0 => e] +// MCDC-DAG: %[[C:.+]] = load i32, ptr %c.addr, align 4 +// MCDC-DAG: %[[CB:.+]] = icmp ne i32 %[[C]], 0 +// MCDC-DAG: %[[CN:.+]] = xor i1 %[[CB]], true +// MCDC-DAG: %[[M2:.+]] = load i32, ptr %mcdc.addr, align 4 +// MCDC-DAG: %[[M2T:.+]] = add i32 %[[M2]], 0 +// MCDC-DAG: %[[M2F:.+]] = add i32 %[[M2]], 0 +// MCDC-DAG: %[[M2S:.+]] = select i1 %[[CN]], i32 %[[M2T]], i32 %[[M2F]] +// MCDC-DAG: store i32 %[[M2S]], ptr %mcdc.addr, align 4 + +// Branch #6, (#5 - #6) [5,0,4] +// d [+8 => END][+1 => e]] +// MCDC-DAG: %[[D:.+]] = load i32, ptr %d.addr, align 4 +// MCDC-DAG: %[[DB:.+]] = icmp ne i32 %[[D]], 0 +// MCDC-DAG: %[[M5:.+]] = load i32, ptr %mcdc.addr, align 4 +// MCDC-DAG: %[[M5T:.+]] = add i32 %[[M5]], 8 +// MCDC-DAG: %[[M5F:.+]] = add i32 %[[M5]], 1 +// MCDC-DAG: %[[M5S:.+]] = select i1 %[[DB]], i32 %[[M5T]], i32 %[[M5F]] +// MCDC-DAG: store i32 %[[M5S]], ptr %mcdc.addr, align 4 + +// Branch #7, (#4 - #7) [4,6,0] +// e [+0 => f][+0 => END] +// from: +// [c => +0] +// [d => +1] +// MCDC-DAG: %[[E:.+]] = load i32, ptr %e.addr, align 4 +// MCDC-DAG: %[[EB:.+]] = icmp ne i32 %[[E]], 0 +// MCDC-DAG: %[[M4:.+]] = load i32, ptr %mcdc.addr, align 4 +// MCDC-DAG: %[[M4T:.+]] = add i32 %[[M4]], 0 +// MCDC-DAG: %[[M4F:.+]] = add i32 %[[M4]], 0 +// MCDC-DAG: %[[M4S:.+]] = select i1 %[[EB]], i32 %[[M4T]], i32 %[[M4F]] +// MCDC-DAG: store i32 %[[M4S]], ptr %mcdc.addr, align 4 + +// Branch #8, (#7 - #8) [6,0,0] +// !f [+4 => END][+2 => END] +// MCDC-DAG: %[[F:.+]] = load i32, ptr %f.addr, align 4 +// MCDC-DAG: %[[FB:.+]] = icmp ne i32 %[[F]], 0 +// MCDC-DAG: %[[FN:.+]] = xor i1 %[[FB]], true +// MCDC-DAG: %[[M6:.+]] = load i32, ptr %mcdc.addr, align 4 +// MCDC-DAG: %[[M6T:.+]] = add i32 %[[M6]], 4 +// MCDC-DAG: %[[M6F:.+]] = add i32 %[[M6]], 2 +// MCDC-DAG: %[[M6S:.+]] = select i1 %[[FN]], i32 %[[M6T]], i32 %[[M6F]] +// MCDC-DAG: store i32 %[[M6S]], ptr %mcdc.addr, align 4 + +// from: +// [e => +0] +// [f => +2] +// [c => +0] +// [d => +1] +// [f => +4] +// [c => +0] +// [d => +1] +// [a => +6] +// [b => +7] +// [d => +8] +// MCDC-DAG: %[[T0:.+]] = load i32, ptr %mcdc.addr, align 4 +// MCDC-DAG: %[[T:.+]] = add i32 %[[T0]], 0 +// MCDC-DAG: %[[TA:.+]] = lshr i32 %[[T]], 3 +// MCDC-DAG: %[[BA:.+]] = getelementptr inbounds i8, ptr @__profbm_internot, i32 %[[TA]] +// MCDC-DAG: %[[BI:.+]] = and i32 %[[T]], 7 +// MCDC-DAG: %[[BI1:.+]] = trunc i32 %[[BI]] to i8 +// MCDC-DAG: %[[BM:.+]] = shl i8 1, %[[BI1]] +// MCDC-DAG: %mcdc.bits = load i8, ptr %[[BA]], align 1 +// MCDC-DAG: %[[BN:.+]] = or i8 %mcdc.bits, %[[BM]] +// MCDC-DAG: store i8 %[[BN]], ptr %[[BA]], align 1 `````````` </details> https://github.com/llvm/llvm-project/pull/125406 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
