Skip to content

Commit f2cf50e

Browse files
chapunichapuni
committed
[MC/DC] Enable usage of ! among && and ||
In the current implementation, `!(a || b) && c` was not treated as one Decision with three terms. Fixes #124563
1 parent a6d4be0 commit f2cf50e

File tree

6 files changed

+132
-9
lines changed

6 files changed

+132
-9
lines changed

‎clang/include/clang/AST/IgnoreExpr.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,14 @@ inline Expr *IgnoreElidableImplicitConstructorSingleStep(Expr *E) {
134134
return E;
135135
}
136136

137+
inline Expr *IgnoreUOpLNotSingleStep(Expr *E) {
138+
if (auto *UO = dyn_cast<UnaryOperator>(E)) {
139+
if (UO->getOpcode() == UO_LNot)
140+
return UO->getSubExpr();
141+
}
142+
return E;
143+
}
144+
137145
inline Expr *IgnoreImplicitAsWrittenSingleStep(Expr *E) {
138146
if (auto *ICE = dyn_cast<ImplicitCastExpr>(E))
139147
return ICE->getSubExprAsWritten();

‎clang/lib/CodeGen/CodeGenFunction.cpp‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "clang/AST/Decl.h"
2828
#include "clang/AST/DeclCXX.h"
2929
#include "clang/AST/Expr.h"
30+
#include "clang/AST/IgnoreExpr.h"
3031
#include "clang/AST/StmtCXX.h"
3132
#include "clang/AST/StmtObjC.h"
3233
#include "clang/Basic/Builtins.h"
@@ -1748,12 +1749,13 @@ bool CodeGenFunction::ConstantFoldsToSimpleInteger(const Expr *Cond,
17481749

17491750
/// Strip parentheses and simplistic logical-NOT operators.
17501751
const Expr *CodeGenFunction::stripCond(const Expr *C) {
1751-
while (const UnaryOperator *Op = dyn_cast<UnaryOperator>(C->IgnoreParens())) {
1752-
if (Op->getOpcode() != UO_LNot)
1753-
break;
1754-
C = Op->getSubExpr();
1752+
while (true) {
1753+
const Expr *SC =
1754+
IgnoreExprNodes(C, IgnoreParensSingleStep, IgnoreUOpLNotSingleStep);
1755+
if (C == SC)
1756+
return SC;
1757+
C = SC;
17551758
}
1756-
return C->IgnoreParens();
17571759
}
17581760

17591761
/// Determine whether the given condition is an instrumentable condition

‎clang/lib/CodeGen/CodeGenPGO.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,9 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
247247
}
248248

249249
if (const Expr *E = dyn_cast<Expr>(S)) {
250-
const BinaryOperator *BinOp = dyn_cast<BinaryOperator>(E->IgnoreParens());
251-
if (BinOp && BinOp->isLogicalOp()) {
250+
if (const auto *BinOp =
251+
dyn_cast<BinaryOperator>(CodeGenFunction::stripCond(E));
252+
BinOp && BinOp->isLogicalOp()) {
252253
/// Check for "split-nested" logical operators. This happens when a new
253254
/// boolean expression logical-op nest is encountered within an existing
254255
/// boolean expression, separated by a non-logical operator. For
@@ -280,7 +281,8 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
280281
return true;
281282

282283
if (const Expr *E = dyn_cast<Expr>(S)) {
283-
const BinaryOperator *BinOp = dyn_cast<BinaryOperator>(E->IgnoreParens());
284+
const BinaryOperator *BinOp =
285+
dyn_cast<BinaryOperator>(CodeGenFunction::stripCond(E));
284286
if (BinOp && BinOp->isLogicalOp()) {
285287
assert(LogOpStack.back() == BinOp);
286288
LogOpStack.pop_back();

‎clang/lib/CodeGen/CoverageMappingGen.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,12 @@ struct MCDCCoverageBuilder {
799799
/// Return the LHS Decision ([0,0] if not set).
800800
const mcdc::ConditionIDs &back() const { return DecisionStack.back(); }
801801

802+
void swapConds() {
803+
if (DecisionStack.empty())
804+
return;
805+
806+
std::swap(DecisionStack.back()[false], DecisionStack.back()[true]);
807+
}
802808
/// Push the binary operator statement to track the nest level and assign IDs
803809
/// to the operator's LHS and RHS. The RHS may be a larger subtree that is
804810
/// broken up on successive levels.
@@ -2241,6 +2247,12 @@ struct CounterCoverageMappingBuilder
22412247
SM.isInSystemHeader(SM.getSpellingLoc(E->getEndLoc())));
22422248
}
22432249

2250+
void VisitUnaryLNot(const UnaryOperator *E) {
2251+
MCDCBuilder.swapConds();
2252+
Visit(E->getSubExpr());
2253+
MCDCBuilder.swapConds();
2254+
}
2255+
22442256
void VisitBinLAnd(const BinaryOperator *E) {
22452257
if (isExprInSystemHeader(E)) {
22462258
LeafExprSet.insert(E);

‎clang/test/CoverageMapping/mcdc-nested-expr.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ bool func_expect(bool a, bool b, bool c) {
2929
// Doesn't split exprs.
3030
// CHECK: func_lnot{{.*}}:
3131
bool func_lnot(bool a, bool b, bool c, bool d) {
32-
// WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression.
3332
return !(a || b) && !(c && d);
33+
// CHECK: Decision,File 0, [[@LINE-1]]:10 -> [[#L:@LINE-1]]:32 = M:5, C:4
34+
// CHECK: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = (#0 - #2), #2 [1,0,3]
35+
// CHECK: Branch,File 0, [[#L]]:17 -> [[#L]]:18 = (#2 - #3), #3 [3,0,2]
36+
// CHECK: Branch,File 0, [[#L]]:25 -> [[#L]]:26 = #4, (#1 - #4) [2,4,0]
37+
// CHECK: Branch,File 0, [[#L]]:30 -> [[#L]]:31 = #5, (#4 - #5) [4,0,0]
3438
}

‎clang/test/Profile/c-mcdc-not.c‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,98 @@ int test(int a, int b, int c, int d, int e, int f) {
8585
// MCDC: %[[BITS:.+]] = load i8, ptr %[[LAB4]], align 1
8686
// MCDC: %[[LAB8:[0-9]+]] = or i8 %[[BITS]], %[[LAB7]]
8787
// MCDC: store i8 %[[LAB8]], ptr %[[LAB4]], align 1
88+
89+
int internot(int a, int b, int c, int d, int e, int f) {
90+
return !(!(!a && b) || !(!!(!c && d) || !(e && !f)));
91+
}
92+
93+
// MCDC-LABEL: @internot(
94+
// MCDC-DAG: store i32 0, ptr %mcdc.addr, align 4
95+
96+
// Branch #2, (#0 - #2) [1,3,0]
97+
// !a [+0 => b][+6 => END]
98+
// MCDC-DAG: %[[A:.+]] = load i32, ptr %a.addr, align 4
99+
// MCDC-DAG: %[[AB:.+]] = icmp ne i32 %[[A]], 0
100+
// MCDC-DAG: %[[AN:.+]] = xor i1 %[[AB]], true
101+
// MCDC-DAG: %[[M1:.+]] = load i32, ptr %mcdc.addr, align 4
102+
// MCDC-DAG: %[[M1T:.+]] = add i32 %[[M1]], 0
103+
// MCDC-DAG: %[[M1F:.+]] = add i32 %[[M1]], 6
104+
// MCDC-DAG: %[[M1S:.+]] = select i1 %[[AN]], i32 %[[M1T]], i32 %[[M1F]]
105+
// MCDC-DAG: store i32 %[[M1S]], ptr %mcdc.addr, align 4
106+
107+
// Branch #3, (#2 - #3) [3,2,0]
108+
// b [+0 => c][+7 => END]
109+
// MCDC-DAG: %[[B:.+]] = load i32, ptr %b.addr, align 4
110+
// MCDC-DAG: %[[BB:.+]] = icmp ne i32 %[[B]], 0
111+
// MCDC-DAG: %[[M3:.+]] = load i32, ptr %mcdc.addr, align 4
112+
// MCDC-DAG: %[[M3T:.+]] = add i32 %[[M3]], 0
113+
// MCDC-DAG: %[[M3F:.+]] = add i32 %[[M3]], 7
114+
// MCDC-DAG: %[[M3S:.+]] = select i1 %[[BB]], i32 %[[M3T]], i32 %[[M3F]]
115+
// MCDC-DAG: store i32 %[[M3S]], ptr %mcdc.addr, align 4
116+
117+
// Branch #5, (#1 - #5) [2,5,4]
118+
// !c [+0 => d][+0 => e]
119+
// MCDC-DAG: %[[C:.+]] = load i32, ptr %c.addr, align 4
120+
// MCDC-DAG: %[[CB:.+]] = icmp ne i32 %[[C]], 0
121+
// MCDC-DAG: %[[CN:.+]] = xor i1 %[[CB]], true
122+
// MCDC-DAG: %[[M2:.+]] = load i32, ptr %mcdc.addr, align 4
123+
// MCDC-DAG: %[[M2T:.+]] = add i32 %[[M2]], 0
124+
// MCDC-DAG: %[[M2F:.+]] = add i32 %[[M2]], 0
125+
// MCDC-DAG: %[[M2S:.+]] = select i1 %[[CN]], i32 %[[M2T]], i32 %[[M2F]]
126+
// MCDC-DAG: store i32 %[[M2S]], ptr %mcdc.addr, align 4
127+
128+
// Branch #6, (#5 - #6) [5,0,4]
129+
// d [+8 => END][+1 => e]]
130+
// MCDC-DAG: %[[D:.+]] = load i32, ptr %d.addr, align 4
131+
// MCDC-DAG: %[[DB:.+]] = icmp ne i32 %[[D]], 0
132+
// MCDC-DAG: %[[M5:.+]] = load i32, ptr %mcdc.addr, align 4
133+
// MCDC-DAG: %[[M5T:.+]] = add i32 %[[M5]], 8
134+
// MCDC-DAG: %[[M5F:.+]] = add i32 %[[M5]], 1
135+
// MCDC-DAG: %[[M5S:.+]] = select i1 %[[DB]], i32 %[[M5T]], i32 %[[M5F]]
136+
// MCDC-DAG: store i32 %[[M5S]], ptr %mcdc.addr, align 4
137+
138+
// Branch #7, (#4 - #7) [4,6,0]
139+
// e [+0 => f][+0 => END]
140+
// from:
141+
// [c => +0]
142+
// [d => +1]
143+
// MCDC-DAG: %[[E:.+]] = load i32, ptr %e.addr, align 4
144+
// MCDC-DAG: %[[EB:.+]] = icmp ne i32 %[[E]], 0
145+
// MCDC-DAG: %[[M4:.+]] = load i32, ptr %mcdc.addr, align 4
146+
// MCDC-DAG: %[[M4T:.+]] = add i32 %[[M4]], 0
147+
// MCDC-DAG: %[[M4F:.+]] = add i32 %[[M4]], 0
148+
// MCDC-DAG: %[[M4S:.+]] = select i1 %[[EB]], i32 %[[M4T]], i32 %[[M4F]]
149+
// MCDC-DAG: store i32 %[[M4S]], ptr %mcdc.addr, align 4
150+
151+
// Branch #8, (#7 - #8) [6,0,0]
152+
// !f [+4 => END][+2 => END]
153+
// MCDC-DAG: %[[F:.+]] = load i32, ptr %f.addr, align 4
154+
// MCDC-DAG: %[[FB:.+]] = icmp ne i32 %[[F]], 0
155+
// MCDC-DAG: %[[FN:.+]] = xor i1 %[[FB]], true
156+
// MCDC-DAG: %[[M6:.+]] = load i32, ptr %mcdc.addr, align 4
157+
// MCDC-DAG: %[[M6T:.+]] = add i32 %[[M6]], 4
158+
// MCDC-DAG: %[[M6F:.+]] = add i32 %[[M6]], 2
159+
// MCDC-DAG: %[[M6S:.+]] = select i1 %[[FN]], i32 %[[M6T]], i32 %[[M6F]]
160+
// MCDC-DAG: store i32 %[[M6S]], ptr %mcdc.addr, align 4
161+
162+
// from:
163+
// [e => +0]
164+
// [f => +2]
165+
// [c => +0]
166+
// [d => +1]
167+
// [f => +4]
168+
// [c => +0]
169+
// [d => +1]
170+
// [a => +6]
171+
// [b => +7]
172+
// [d => +8]
173+
// MCDC-DAG: %[[T0:.+]] = load i32, ptr %mcdc.addr, align 4
174+
// MCDC-DAG: %[[T:.+]] = add i32 %[[T0]], 0
175+
// MCDC-DAG: %[[TA:.+]] = lshr i32 %[[T]], 3
176+
// MCDC-DAG: %[[BA:.+]] = getelementptr inbounds i8, ptr @__profbm_internot, i32 %[[TA]]
177+
// MCDC-DAG: %[[BI:.+]] = and i32 %[[T]], 7
178+
// MCDC-DAG: %[[BI1:.+]] = trunc i32 %[[BI]] to i8
179+
// MCDC-DAG: %[[BM:.+]] = shl i8 1, %[[BI1]]
180+
// MCDC-DAG: %mcdc.bits = load i8, ptr %[[BA]], align 1
181+
// MCDC-DAG: %[[BN:.+]] = or i8 %mcdc.bits, %[[BM]]
182+
// MCDC-DAG: store i8 %[[BN]], ptr %[[BA]], align 1

0 commit comments

Comments
 (0)