[clang] [llvm] [clang][CoverageMapping] Refactor when setting MC/DC True/False (PR #78202)
Alan Phipps via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 16 12:30:49 PST 2024
https://github.com/evodius96 updated https://github.com/llvm/llvm-project/pull/78202
>From 8751417f4889a120193a604e2ea91627f3b064e8 Mon Sep 17 00:00:00 2001
From: Alan Phipps <a-phipps at ti.com>
Date: Mon, 15 Jan 2024 12:24:36 -0600
Subject: [PATCH 1/3] [clang][CoverageMapping] Refactor when setting MC/DC
True/False Condition IDs
Clean-up of the algorithm that assigns MC/DC True/False control-flow
condition IDs when constructing an MC/DC decision region. This patch creates a
common API for setting/getting the condition IDs, making the binary logical
operator visitor functions much cleaner.
This patch also fixes a bug in which the decision which was malformed due to an
incorrect calculation of the True/False condition IDs. Previously, this
calculation was done as the logical operation stack was being popped, yielding
off-by-one errors. This is now done when the logical operation stack is being
pushed.
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 127 +++++++++++-------
.../mcdc-logical-stmt-ids-all.cpp | 50 +++++++
.../ProfileData/Coverage/CoverageMapping.h | 2 +-
3 files changed, 133 insertions(+), 46 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 916016601a93276..ca7b65e3f4b08d7 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -668,6 +668,8 @@ struct MCDCCoverageBuilder {
llvm::SmallVector<const BinaryOperator *> NestLevel;
llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDs;
llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap;
+ llvm::DenseMap<const Stmt *, MCDCConditionID> TrueCondIDs;
+ llvm::DenseMap<const Stmt *, MCDCConditionID> FalseCondIDs;
MCDCConditionID NextID = 1;
bool NotMapped = false;
@@ -713,6 +715,21 @@ struct MCDCCoverageBuilder {
return AndRHS.empty() ? 0 : AndRHS.back();
}
+ /// Set the given condition's ID.
+ void setCondID(const Expr *Cond, MCDCConditionID ID) {
+ CondIDs[CodeGenFunction::stripCond(Cond)] = ID;
+ }
+
+ /// Set the ID of the next condition when the given condition is True.
+ void setTrueCondID(const Expr *Cond, MCDCConditionID ID) {
+ TrueCondIDs[CodeGenFunction::stripCond(Cond)] = ID;
+ }
+
+ /// Set the ID of the next condition when the given condition is False.
+ void setFalseCondID(const Expr *Cond, MCDCConditionID ID) {
+ FalseCondIDs[CodeGenFunction::stripCond(Cond)] = ID;
+ }
+
/// Return the ID of a given condition.
MCDCConditionID getCondID(const Expr *Cond) const {
auto I = CondIDs.find(CodeGenFunction::stripCond(Cond));
@@ -722,6 +739,24 @@ struct MCDCCoverageBuilder {
return I->second;
}
+ /// Return the ID of the next condition when the given condition is True.
+ MCDCConditionID getNextIfTrueCondID(const Expr *Cond) const {
+ auto I = TrueCondIDs.find(CodeGenFunction::stripCond(Cond));
+ if (I == TrueCondIDs.end())
+ return 0;
+ else
+ return I->second;
+ }
+
+ /// Return the ID of the next condition when the given condition is False.
+ MCDCConditionID getNextIfFalseCondID(const Expr *Cond) const {
+ auto I = FalseCondIDs.find(CodeGenFunction::stripCond(Cond));
+ if (I == FalseCondIDs.end())
+ return 0;
+ else
+ return I->second;
+ }
+
/// 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.
@@ -746,7 +781,7 @@ struct MCDCCoverageBuilder {
// assign that ID to its LHS node. Its RHS will receive a new ID.
if (CondIDs.contains(CodeGenFunction::stripCond(E))) {
// If Stmt has an ID, assign its ID to LHS
- CondIDs[CodeGenFunction::stripCond(E->getLHS())] = CondIDs[E];
+ setCondID(E->getLHS(), getCondID(E));
// Since the operator's LHS assumes the operator's same ID, pop the
// operator from the RHS stack so that if LHS short-circuits, it won't be
@@ -754,13 +789,44 @@ struct MCDCCoverageBuilder {
popRHSifTop(E);
} else {
// Otherwise, assign ID+1 to LHS.
- CondIDs[CodeGenFunction::stripCond(E->getLHS())] = NextID++;
+ setCondID(E->getLHS(), NextID++);
}
// Assign ID+1 to RHS.
- CondIDs[CodeGenFunction::stripCond(E->getRHS())] = NextID++;
+ setCondID(E->getRHS(), NextID++);
+
+ // Assign the True and False condition IDs for the LHS and RHS.
+ if (isLAnd(E)) {
+ // For logical-AND ("LHS && RHS"):
+ // - If LHS is TRUE, execution goes to the RHS node.
+ // - If LHS is FALSE, execution goes to the LHS node of the next LOr.
+ // If that does not exist, execution exits (ID == 0).
+ setTrueCondID(E->getLHS(), getCondID(E->getRHS()));
+ setFalseCondID(E->getLHS(), getNextLOrCondID());
+
+ // - If RHS is TRUE, execution goes to LHS node of the next LAnd.
+ // If that does not exist, execution exits (ID == 0).
+ // - If RHS is FALSE, execution goes to the LHS node of the next LOr.
+ // If that does not exist, execution exits (ID == 0).
+ setTrueCondID(E->getRHS(), getNextLAndCondID());
+ setFalseCondID(E->getRHS(), getNextLOrCondID());
+ } else {
+ // For logical-OR ("LHS || RHS"):
+ // - If LHS is TRUE, execution goes to the LHS node of the next LAnd.
+ // If that does not exist, execution exits (ID == 0).
+ // - If LHS is FALSE, execution goes to the RHS node.
+ setTrueCondID(E->getLHS(), getNextLAndCondID());
+ setFalseCondID(E->getLHS(), getCondID(E->getRHS()));
+
+ // - If RHS is TRUE, execution goes to LHS node of the next LAnd.
+ // If that does not exist, execution exits (ID == 0).
+ // - If RHS is FALSE, execution goes to the LHS node of the next LOr.
+ // If that does not exist, execution exits (ID == 0).
+ setTrueCondID(E->getRHS(), getNextLAndCondID());
+ setFalseCondID(E->getRHS(), getNextLOrCondID());
+ }
- // Push ID of Stmt's RHS so that LHS nodes know about it
+ // Push ID of Stmt's RHS so that LHS nodes know about it.
pushRHS(E);
}
@@ -1022,9 +1088,7 @@ struct CounterCoverageMappingBuilder
/// and add it to the function's SourceRegions. A branch region tracks a
/// "True" counter and a "False" counter for boolean expressions that
/// result in the generation of a branch.
- void createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt,
- MCDCConditionID ID = 0, MCDCConditionID TrueID = 0,
- MCDCConditionID FalseID = 0) {
+ void createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt) {
// Check for NULL conditions.
if (!C)
return;
@@ -1034,6 +1098,11 @@ struct CounterCoverageMappingBuilder
// function's SourceRegions) because it doesn't apply to any other source
// code other than the Condition.
if (CodeGenFunction::isInstrumentedCondition(C)) {
+ // Extract the MCDC condition IDs (returns 0 if not needed).
+ MCDCConditionID ID = MCDCBuilder.getCondID(C);
+ MCDCConditionID TrueID = MCDCBuilder.getNextIfTrueCondID(C);
+ MCDCConditionID FalseID = MCDCBuilder.getNextIfFalseCondID(C);
+
// If a condition can fold to true or false, the corresponding branch
// will be removed. Create a region with both counters hard-coded to
// zero. This allows us to visualize them in a special way.
@@ -1833,7 +1902,7 @@ struct CounterCoverageMappingBuilder
extendRegion(E->getRHS());
propagateCounts(getRegionCounter(E), E->getRHS());
- // Process Binary Operator and create MCDC Decision Region if top-level
+ // Process Binary Operator and create MCDC Decision Region if top-level.
unsigned NumConds = 0;
if ((NumConds = MCDCBuilder.popAndReturnCondCount(E)))
createDecisionRegion(E, getRegionBitmap(E), NumConds);
@@ -1847,30 +1916,13 @@ struct CounterCoverageMappingBuilder
// Extract the Parent Region Counter.
Counter ParentCnt = getRegion().getCounter();
- // Extract the MCDC condition IDs (returns 0 if not needed).
- MCDCConditionID NextOrID = MCDCBuilder.getNextLOrCondID();
- MCDCConditionID NextAndID = MCDCBuilder.getNextLAndCondID();
- MCDCConditionID LHSid = MCDCBuilder.getCondID(E->getLHS());
- MCDCConditionID RHSid = MCDCBuilder.getCondID(E->getRHS());
-
// Create Branch Region around LHS condition.
- // MC/DC: For "LHS && RHS"
- // - If LHS is TRUE, execution goes to the RHS.
- // - If LHS is FALSE, execution goes to the LHS of the next logical-OR.
- // If that does not exist, execution exits (ID == 0).
createBranchRegion(E->getLHS(), RHSExecCnt,
- subtractCounters(ParentCnt, RHSExecCnt), LHSid, RHSid,
- NextOrID);
+ subtractCounters(ParentCnt, RHSExecCnt));
// Create Branch Region around RHS condition.
- // MC/DC: For "LHS && RHS"
- // - If RHS is TRUE, execution goes to LHS of the next logical-AND.
- // If that does not exist, execution exits (ID == 0).
- // - If RHS is FALSE, execution goes to the LHS of the next logical-OR.
- // If that does not exist, execution exits (ID == 0).
createBranchRegion(E->getRHS(), RHSTrueCnt,
- subtractCounters(RHSExecCnt, RHSTrueCnt), RHSid,
- NextAndID, NextOrID);
+ subtractCounters(RHSExecCnt, RHSTrueCnt));
}
// Determine whether the right side of OR operation need to be visited.
@@ -1895,7 +1947,7 @@ struct CounterCoverageMappingBuilder
extendRegion(E->getRHS());
propagateCounts(getRegionCounter(E), E->getRHS());
- // Process Binary Operator and create MCDC Decision Region if top-level
+ // Process Binary Operator and create MCDC Decision Region if top-level.
unsigned NumConds = 0;
if ((NumConds = MCDCBuilder.popAndReturnCondCount(E)))
createDecisionRegion(E, getRegionBitmap(E), NumConds);
@@ -1913,28 +1965,13 @@ struct CounterCoverageMappingBuilder
// Extract the Parent Region Counter.
Counter ParentCnt = getRegion().getCounter();
- // Extract the MCDC condition IDs (returns 0 if not needed).
- MCDCConditionID NextOrID = MCDCBuilder.getNextLOrCondID();
- MCDCConditionID NextAndID = MCDCBuilder.getNextLAndCondID();
- MCDCConditionID LHSid = MCDCBuilder.getCondID(E->getLHS());
- MCDCConditionID RHSid = MCDCBuilder.getCondID(E->getRHS());
-
// Create Branch Region around LHS condition.
- // MC/DC: For "LHS || RHS"
- // - If LHS is TRUE, execution goes to the LHS of the next logical-AND.
- // If that does not exist, execution exits (ID == 0).
- // - If LHS is FALSE, execution goes to the RHS.
createBranchRegion(E->getLHS(), subtractCounters(ParentCnt, RHSExecCnt),
- RHSExecCnt, LHSid, NextAndID, RHSid);
+ RHSExecCnt);
// Create Branch Region around RHS condition.
- // MC/DC: For "LHS || RHS"
- // - If RHS is TRUE, execution goes to LHS of the next logical-AND.
- // If that does not exist, execution exits (ID == 0).
- // - If RHS is FALSE, execution goes to the LHS of the next logical-OR.
- // If that does not exist, execution exits (ID == 0).
createBranchRegion(E->getRHS(), subtractCounters(RHSExecCnt, RHSFalseCnt),
- RHSFalseCnt, RHSid, NextAndID, NextOrID);
+ RHSFalseCnt);
}
void VisitLambdaExpr(const LambdaExpr *LE) {
diff --git a/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp b/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp
index 5b13cc3507e96cd..b1d32bdd746cb40 100644
--- a/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp
+++ b/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp
@@ -129,3 +129,53 @@ bool func_ternary_or(bool a, bool b, bool c, bool d, bool e, bool f) {
// CHECK: Branch,File 0, 122:26 -> 122:27 = (#6 - #7), #7 [4,0,3]
// CHECK: Branch,File 0, 122:31 -> 122:32 = (#4 - #5), #5 [3,0,2]
// CHECK: Branch,File 0, 122:36 -> 122:37 = (#2 - #3), #3 [2,0,0]
+
+bool func_if_nested_and_if(bool a, bool b, bool c, bool d, bool e) {
+ if (a || (b && c) || d || e)
+ return true;
+ else
+ return false;
+}
+
+// CHECK-LABEL: Decision,File 0, 134:7 -> 134:30 = M:0, C:5
+// CHECK-NEXT: Branch,File 0, 134:7 -> 134:8 = (#0 - #6), #6 [1,0,4]
+// CHECK: Branch,File 0, 134:13 -> 134:14 = #7, (#6 - #7) [4,5,3]
+// CHECK: Branch,File 0, 134:18 -> 134:19 = #8, (#7 - #8) [5,0,3]
+// CHECK: Branch,File 0, 134:24 -> 134:25 = (#4 - #5), #5 [3,0,2]
+// CHECK: Branch,File 0, 134:29 -> 134:30 = (#2 - #3), #3 [2,0,0]
+
+bool func_ternary_nested_and_if(bool a, bool b, bool c, bool d, bool e) {
+ return (a || (b && c) || d || e) ? true : false;
+}
+
+// CHECK-LABEL: Decision,File 0, 148:11 -> 148:34 = M:0, C:5
+// CHECK-NEXT: Branch,File 0, 148:11 -> 148:12 = (#0 - #6), #6 [1,0,4]
+// CHECK: Branch,File 0, 148:17 -> 148:18 = #7, (#6 - #7) [4,5,3]
+// CHECK: Branch,File 0, 148:22 -> 148:23 = #8, (#7 - #8) [5,0,3]
+// CHECK: Branch,File 0, 148:28 -> 148:29 = (#4 - #5), #5 [3,0,2]
+// CHECK: Branch,File 0, 148:33 -> 148:34 = (#2 - #3), #3 [2,0,0]
+
+bool func_if_nested_and_if_2(bool a, bool b, bool c, bool d, bool e) {
+ if (a || ((b && c) || d) && e)
+ return true;
+ else
+ return false;
+}
+
+// CHECK-LABEL: Decision,File 0, 159:7 -> 159:32 = M:0, C:5
+// CHECK-NEXT: Branch,File 0, 159:7 -> 159:8 = (#0 - #2), #2 [1,0,2]
+// CHECK: Branch,File 0, 159:14 -> 159:15 = #7, (#2 - #7) [2,5,4]
+// CHECK: Branch,File 0, 159:19 -> 159:20 = #8, (#7 - #8) [5,3,4]
+// CHECK: Branch,File 0, 159:25 -> 159:26 = (#5 - #6), #6 [4,3,0]
+// CHECK: Branch,File 0, 159:31 -> 159:32 = #4, (#3 - #4) [3,0,0]
+
+bool func_ternary_nested_and_if_2(bool a, bool b, bool c, bool d, bool e) {
+ return (a || ((b && c) || d) && e) ? true : false;
+}
+
+// CHECK-LABEL: Decision,File 0, 173:11 -> 173:36 = M:0, C:5
+// CHECK-NEXT: Branch,File 0, 173:11 -> 173:12 = (#0 - #2), #2 [1,0,2]
+// CHECK: Branch,File 0, 173:18 -> 173:19 = #7, (#2 - #7) [2,5,4]
+// CHECK: Branch,File 0, 173:23 -> 173:24 = #8, (#7 - #8) [5,3,4]
+// CHECK: Branch,File 0, 173:29 -> 173:30 = (#5 - #6), #6 [4,3,0]
+// CHECK: Branch,File 0, 173:35 -> 173:36 = #4, (#3 - #4) [3,0,0]
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 2757b8cd54a69c5..367c884957ff6d7 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -298,7 +298,7 @@ struct CounterMappingRegion {
unsigned ExpandedFileID, unsigned LineStart,
unsigned ColumnStart, unsigned LineEnd,
unsigned ColumnEnd, RegionKind Kind)
- : MCDCParams(MCDCParams), ExpandedFileID(ExpandedFileID),
+ : MCDCParams(MCDCParams), FileID(FileID), ExpandedFileID(ExpandedFileID),
LineStart(LineStart), ColumnStart(ColumnStart), LineEnd(LineEnd),
ColumnEnd(ColumnEnd), Kind(Kind) {}
>From 768a040ebdb4d2ef446cd7914ee64d69711b8378 Mon Sep 17 00:00:00 2001
From: Alan Phipps <a-phipps at ti.com>
Date: Mon, 15 Jan 2024 12:45:17 -0600
Subject: [PATCH 2/3] Update formatting changes
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index ca7b65e3f4b08d7..499e0e5d04d8631 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -717,17 +717,17 @@ struct MCDCCoverageBuilder {
/// Set the given condition's ID.
void setCondID(const Expr *Cond, MCDCConditionID ID) {
- CondIDs[CodeGenFunction::stripCond(Cond)] = ID;
+ CondIDs[CodeGenFunction::stripCond(Cond)] = ID;
}
/// Set the ID of the next condition when the given condition is True.
void setTrueCondID(const Expr *Cond, MCDCConditionID ID) {
- TrueCondIDs[CodeGenFunction::stripCond(Cond)] = ID;
+ TrueCondIDs[CodeGenFunction::stripCond(Cond)] = ID;
}
/// Set the ID of the next condition when the given condition is False.
void setFalseCondID(const Expr *Cond, MCDCConditionID ID) {
- FalseCondIDs[CodeGenFunction::stripCond(Cond)] = ID;
+ FalseCondIDs[CodeGenFunction::stripCond(Cond)] = ID;
}
/// Return the ID of a given condition.
>From cd516da06fa326229995b442f929781c010e4a27 Mon Sep 17 00:00:00 2001
From: Alan Phipps <a-phipps at ti.com>
Date: Tue, 16 Jan 2024 13:29:32 -0600
Subject: [PATCH 3/3] Cleanup Decision push/pop and Nesting Level Tracking
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 248 +++++++-----------
.../mcdc-logical-stmt-ids-all.cpp | 35 ++-
.../ProfileData/Coverage/CoverageMapping.h | 13 +-
3 files changed, 137 insertions(+), 159 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 499e0e5d04d8631..ad1ee264cd9670c 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -616,14 +616,14 @@ struct MCDCCoverageBuilder {
///
/// A node ID of '0' always means MC/DC isn't being tracked.
///
- /// As the AST walk proceeds recursively, the algorithm will also use stacks
+ /// As the AST walk proceeds recursively, the algorithm will also use a stack
/// to track the IDs of logical-AND and logical-OR operations on the RHS so
/// that it can be determined which nodes are executed next, depending on how
/// a LHS or RHS of a logical-AND or logical-OR is evaluated. This
/// information relies on the assigned IDs and are embedded within the
/// coverage region IDs of each branch region associated with a leaf-level
/// condition. This information helps the visualization tool reconstruct all
- /// possible test vectors for the purposes of MC/DC analysis. if a "next" node
+ /// possible test vectors for the purposes of MC/DC analysis. If a "next" node
/// ID is '0', it means it's the end of the test vector. The following rules
/// are used:
///
@@ -663,13 +663,14 @@ struct MCDCCoverageBuilder {
private:
CodeGenModule &CGM;
- llvm::SmallVector<MCDCConditionID> AndRHS;
- llvm::SmallVector<MCDCConditionID> OrRHS;
- llvm::SmallVector<const BinaryOperator *> NestLevel;
+ struct DecisionIDPair {
+ MCDCConditionID TrueID = 0;
+ MCDCConditionID FalseID = 0;
+ };
+
+ llvm::SmallVector<DecisionIDPair, 1> DecisionStack;
llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDs;
llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap;
- llvm::DenseMap<const Stmt *, MCDCConditionID> TrueCondIDs;
- llvm::DenseMap<const Stmt *, MCDCConditionID> FalseCondIDs;
MCDCConditionID NextID = 1;
bool NotMapped = false;
@@ -678,58 +679,27 @@ struct MCDCCoverageBuilder {
return E->getOpcode() == BO_LAnd;
}
- /// Push an ID onto the corresponding RHS stack.
- void pushRHS(const BinaryOperator *E) {
- llvm::SmallVector<MCDCConditionID> &rhs = isLAnd(E) ? AndRHS : OrRHS;
- rhs.push_back(CondIDs[CodeGenFunction::stripCond(E->getRHS())]);
- }
-
- /// Pop an ID from the corresponding RHS stack.
- void popRHS(const BinaryOperator *E) {
- llvm::SmallVector<MCDCConditionID> &rhs = isLAnd(E) ? AndRHS : OrRHS;
- if (!rhs.empty())
- rhs.pop_back();
- }
-
- /// If the expected ID is on top, pop it off the corresponding RHS stack.
- void popRHSifTop(const BinaryOperator *E) {
- if (!OrRHS.empty() && CondIDs[E] == OrRHS.back())
- OrRHS.pop_back();
- else if (!AndRHS.empty() && CondIDs[E] == AndRHS.back())
- AndRHS.pop_back();
- }
-
public:
MCDCCoverageBuilder(CodeGenModule &CGM,
llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDMap,
llvm::DenseMap<const Stmt *, unsigned> &MCDCBitmapMap)
- : CGM(CGM), CondIDs(CondIDMap), MCDCBitmapMap(MCDCBitmapMap) {}
+ : CGM(CGM), DecisionStack(1), CondIDs(CondIDMap),
+ MCDCBitmapMap(MCDCBitmapMap) {}
- /// Return the ID of the RHS of the next, upper nest-level logical-OR.
- MCDCConditionID getNextLOrCondID() const {
- return OrRHS.empty() ? 0 : OrRHS.back();
- }
+ /// Return whether the control flow map is not presently being built. This
+ /// can be used to determine whether the flow is at the root node of an
+ /// expression if that expression is mapped.
+ bool isIdle() { return (NextID == 1 && !NotMapped); }
- /// Return the ID of the RHS of the next, upper nest-level logical-AND.
- MCDCConditionID getNextLAndCondID() const {
- return AndRHS.empty() ? 0 : AndRHS.back();
- }
+ /// Return whether the control flow map is in the process of being built for
+ /// a mapped expression.
+ bool isBuilding() { return (NextID > 1); }
/// Set the given condition's ID.
void setCondID(const Expr *Cond, MCDCConditionID ID) {
CondIDs[CodeGenFunction::stripCond(Cond)] = ID;
}
- /// Set the ID of the next condition when the given condition is True.
- void setTrueCondID(const Expr *Cond, MCDCConditionID ID) {
- TrueCondIDs[CodeGenFunction::stripCond(Cond)] = ID;
- }
-
- /// Set the ID of the next condition when the given condition is False.
- void setFalseCondID(const Expr *Cond, MCDCConditionID ID) {
- FalseCondIDs[CodeGenFunction::stripCond(Cond)] = ID;
- }
-
/// Return the ID of a given condition.
MCDCConditionID getCondID(const Expr *Cond) const {
auto I = CondIDs.find(CodeGenFunction::stripCond(Cond));
@@ -739,22 +709,10 @@ struct MCDCCoverageBuilder {
return I->second;
}
- /// Return the ID of the next condition when the given condition is True.
- MCDCConditionID getNextIfTrueCondID(const Expr *Cond) const {
- auto I = TrueCondIDs.find(CodeGenFunction::stripCond(Cond));
- if (I == TrueCondIDs.end())
- return 0;
- else
- return I->second;
- }
-
- /// Return the ID of the next condition when the given condition is False.
- MCDCConditionID getNextIfFalseCondID(const Expr *Cond) const {
- auto I = FalseCondIDs.find(CodeGenFunction::stripCond(Cond));
- if (I == FalseCondIDs.end())
- return 0;
- else
- return I->second;
+ /// Return the LHS Decision ({0,0} if not set).
+ const DecisionIDPair &back() {
+ assert(DecisionStack.size() >= 1);
+ return DecisionStack.back();
}
/// Push the binary operator statement to track the nest level and assign IDs
@@ -765,99 +723,70 @@ struct MCDCCoverageBuilder {
return;
// If binary expression is disqualified, don't do mapping.
- if (NestLevel.empty() &&
- !MCDCBitmapMap.contains(CodeGenFunction::stripCond(E)))
+ if (!isBuilding() && !MCDCBitmapMap.contains(CodeGenFunction::stripCond(E)))
NotMapped = true;
- // Push Stmt on 'NestLevel' stack to keep track of nest location.
- NestLevel.push_back(E);
-
// Don't go any further if we don't need to map condition IDs.
if (NotMapped)
return;
+ assert(DecisionStack.size() >= 1);
+ const DecisionIDPair &ParentDecision = DecisionStack.back();
+
// If the operator itself has an assigned ID, this means it represents a
- // larger subtree. In this case, pop its ID out of the RHS stack and
- // assign that ID to its LHS node. Its RHS will receive a new ID.
- if (CondIDs.contains(CodeGenFunction::stripCond(E))) {
- // If Stmt has an ID, assign its ID to LHS
+ // larger subtree. In this case, assign that ID to its LHS node. Its RHS
+ // will receive a new ID below. Otherwise, assign ID+1 to LHS.
+ if (CondIDs.contains(CodeGenFunction::stripCond(E)))
setCondID(E->getLHS(), getCondID(E));
-
- // Since the operator's LHS assumes the operator's same ID, pop the
- // operator from the RHS stack so that if LHS short-circuits, it won't be
- // incorrectly re-used as the node executed next.
- popRHSifTop(E);
- } else {
- // Otherwise, assign ID+1 to LHS.
+ else
setCondID(E->getLHS(), NextID++);
- }
- // Assign ID+1 to RHS.
- setCondID(E->getRHS(), NextID++);
-
- // Assign the True and False condition IDs for the LHS and RHS.
- if (isLAnd(E)) {
- // For logical-AND ("LHS && RHS"):
- // - If LHS is TRUE, execution goes to the RHS node.
- // - If LHS is FALSE, execution goes to the LHS node of the next LOr.
- // If that does not exist, execution exits (ID == 0).
- setTrueCondID(E->getLHS(), getCondID(E->getRHS()));
- setFalseCondID(E->getLHS(), getNextLOrCondID());
-
- // - If RHS is TRUE, execution goes to LHS node of the next LAnd.
- // If that does not exist, execution exits (ID == 0).
- // - If RHS is FALSE, execution goes to the LHS node of the next LOr.
- // If that does not exist, execution exits (ID == 0).
- setTrueCondID(E->getRHS(), getNextLAndCondID());
- setFalseCondID(E->getRHS(), getNextLOrCondID());
- } else {
- // For logical-OR ("LHS || RHS"):
- // - If LHS is TRUE, execution goes to the LHS node of the next LAnd.
- // If that does not exist, execution exits (ID == 0).
- // - If LHS is FALSE, execution goes to the RHS node.
- setTrueCondID(E->getLHS(), getNextLAndCondID());
- setFalseCondID(E->getLHS(), getCondID(E->getRHS()));
-
- // - If RHS is TRUE, execution goes to LHS node of the next LAnd.
- // If that does not exist, execution exits (ID == 0).
- // - If RHS is FALSE, execution goes to the LHS node of the next LOr.
- // If that does not exist, execution exits (ID == 0).
- setTrueCondID(E->getRHS(), getNextLAndCondID());
- setFalseCondID(E->getRHS(), getNextLOrCondID());
- }
+ // Assign a ID+1 for the RHS.
+ MCDCConditionID RHSid = NextID++;
+ setCondID(E->getRHS(), RHSid);
+
+ // Push the LHS decision IDs onto the DecisionStack.
+ if (isLAnd(E))
+ DecisionStack.push_back({RHSid, ParentDecision.FalseID});
+ else
+ DecisionStack.push_back({ParentDecision.TrueID, RHSid});
+ }
+
+ /// Pop and return the LHS Decision ({0,0} if not set).
+ DecisionIDPair pop() {
+ assert(DecisionStack.size() >= 1);
+
+ if (!CGM.getCodeGenOpts().MCDCCoverage || NotMapped)
+ return DecisionStack.front();
- // Push ID of Stmt's RHS so that LHS nodes know about it.
- pushRHS(E);
+ assert(DecisionStack.size() > 1);
+ DecisionIDPair D = DecisionStack.back();
+ DecisionStack.pop_back();
+ return D;
}
- /// Pop the binary operator from the next level. If the walk is at the top of
- /// the next, assign the total number of conditions.
- unsigned popAndReturnCondCount(const BinaryOperator *E) {
+ /// Return the total number of conditions and reset the state. The number of
+ /// conditions is zero if the expression isn't mapped.
+ unsigned getTotalConditionsAndReset(const BinaryOperator *E) {
if (!CGM.getCodeGenOpts().MCDCCoverage)
return 0;
- unsigned TotalConds = 0;
-
- // Pop Stmt from 'NestLevel' stack.
- assert(NestLevel.back() == E);
- NestLevel.pop_back();
+ assert(!isIdle());
+ assert(DecisionStack.size() == 1);
// Reset state if not doing mapping.
- if (NestLevel.empty() && NotMapped) {
+ if (NotMapped) {
NotMapped = false;
+ assert(NextID == 1);
return 0;
}
- // Pop RHS ID.
- popRHS(E);
+ // Set number of conditions and reset.
+ unsigned TotalConds = NextID - 1;
- // If at the parent (NestLevel=0), set conds and reset.
- if (NestLevel.empty()) {
- TotalConds = NextID - 1;
+ // Reset ID back to beginning.
+ NextID = 1;
- // Reset ID back to beginning.
- NextID = 1;
- }
return TotalConds;
}
};
@@ -1088,7 +1017,9 @@ struct CounterCoverageMappingBuilder
/// and add it to the function's SourceRegions. A branch region tracks a
/// "True" counter and a "False" counter for boolean expressions that
/// result in the generation of a branch.
- void createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt) {
+ void createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt,
+ MCDCConditionID ID = 0, MCDCConditionID TrueID = 0,
+ MCDCConditionID FalseID = 0) {
// Check for NULL conditions.
if (!C)
return;
@@ -1098,11 +1029,6 @@ struct CounterCoverageMappingBuilder
// function's SourceRegions) because it doesn't apply to any other source
// code other than the Condition.
if (CodeGenFunction::isInstrumentedCondition(C)) {
- // Extract the MCDC condition IDs (returns 0 if not needed).
- MCDCConditionID ID = MCDCBuilder.getCondID(C);
- MCDCConditionID TrueID = MCDCBuilder.getNextIfTrueCondID(C);
- MCDCConditionID FalseID = MCDCBuilder.getNextIfFalseCondID(C);
-
// If a condition can fold to true or false, the corresponding branch
// will be removed. Create a region with both counters hard-coded to
// zero. This allows us to visualize them in a special way.
@@ -1891,20 +1817,28 @@ struct CounterCoverageMappingBuilder
}
void VisitBinLAnd(const BinaryOperator *E) {
- // Keep track of Binary Operator and assign MCDC condition IDs
+ bool IsRootNode = MCDCBuilder.isIdle();
+
+ // Keep track of Binary Operator and assign MCDC condition IDs.
MCDCBuilder.pushAndAssignIDs(E);
extendRegion(E->getLHS());
propagateCounts(getRegion().getCounter(), E->getLHS());
handleFileExit(getEnd(E->getLHS()));
+ // Track LHS True/False Decision.
+ auto DecisionLHS = MCDCBuilder.pop();
+
// Counter tracks the right hand side of a logical and operator.
extendRegion(E->getRHS());
propagateCounts(getRegionCounter(E), E->getRHS());
- // Process Binary Operator and create MCDC Decision Region if top-level.
+ // Track RHS True/False Decision.
+ const auto &DecisionRHS = MCDCBuilder.back();
+
+ // Create MCDC Decision Region if at top-level (root).
unsigned NumConds = 0;
- if ((NumConds = MCDCBuilder.popAndReturnCondCount(E)))
+ if (IsRootNode && (NumConds = MCDCBuilder.getTotalConditionsAndReset(E)))
createDecisionRegion(E, getRegionBitmap(E), NumConds);
// Extract the RHS's Execution Counter.
@@ -1916,13 +1850,18 @@ struct CounterCoverageMappingBuilder
// Extract the Parent Region Counter.
Counter ParentCnt = getRegion().getCounter();
+ MCDCConditionID LHSid = MCDCBuilder.getCondID(E->getLHS());
+ MCDCConditionID RHSid = MCDCBuilder.getCondID(E->getRHS());
+
// Create Branch Region around LHS condition.
createBranchRegion(E->getLHS(), RHSExecCnt,
- subtractCounters(ParentCnt, RHSExecCnt));
+ subtractCounters(ParentCnt, RHSExecCnt), LHSid,
+ DecisionLHS.TrueID, DecisionLHS.FalseID);
// Create Branch Region around RHS condition.
createBranchRegion(E->getRHS(), RHSTrueCnt,
- subtractCounters(RHSExecCnt, RHSTrueCnt));
+ subtractCounters(RHSExecCnt, RHSTrueCnt), RHSid,
+ DecisionRHS.TrueID, DecisionRHS.FalseID);
}
// Determine whether the right side of OR operation need to be visited.
@@ -1936,20 +1875,28 @@ struct CounterCoverageMappingBuilder
}
void VisitBinLOr(const BinaryOperator *E) {
- // Keep track of Binary Operator and assign MCDC condition IDs
+ bool IsRootNode = MCDCBuilder.isIdle();
+
+ // Keep track of Binary Operator and assign MCDC condition IDs.
MCDCBuilder.pushAndAssignIDs(E);
extendRegion(E->getLHS());
Counter OutCount = propagateCounts(getRegion().getCounter(), E->getLHS());
handleFileExit(getEnd(E->getLHS()));
+ // Track LHS True/False Decision.
+ auto DecisionLHS = MCDCBuilder.pop();
+
// Counter tracks the right hand side of a logical or operator.
extendRegion(E->getRHS());
propagateCounts(getRegionCounter(E), E->getRHS());
- // Process Binary Operator and create MCDC Decision Region if top-level.
+ // Track RHS True/False Decision.
+ const auto &DecisionRHS = MCDCBuilder.back();
+
+ // Create MCDC Decision Region if at top-level (root).
unsigned NumConds = 0;
- if ((NumConds = MCDCBuilder.popAndReturnCondCount(E)))
+ if (IsRootNode && (NumConds = MCDCBuilder.getTotalConditionsAndReset(E)))
createDecisionRegion(E, getRegionBitmap(E), NumConds);
// Extract the RHS's Execution Counter.
@@ -1965,13 +1912,18 @@ struct CounterCoverageMappingBuilder
// Extract the Parent Region Counter.
Counter ParentCnt = getRegion().getCounter();
+ MCDCConditionID LHSid = MCDCBuilder.getCondID(E->getLHS());
+ MCDCConditionID RHSid = MCDCBuilder.getCondID(E->getRHS());
+
// Create Branch Region around LHS condition.
createBranchRegion(E->getLHS(), subtractCounters(ParentCnt, RHSExecCnt),
- RHSExecCnt);
+ RHSExecCnt, LHSid, DecisionLHS.TrueID,
+ DecisionLHS.FalseID);
// Create Branch Region around RHS condition.
createBranchRegion(E->getRHS(), subtractCounters(RHSExecCnt, RHSFalseCnt),
- RHSFalseCnt);
+ RHSFalseCnt, RHSid, DecisionRHS.TrueID,
+ DecisionRHS.FalseID);
}
void VisitLambdaExpr(const LambdaExpr *LE) {
diff --git a/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp b/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp
index b1d32bdd746cb40..6f47a4b901a8a75 100644
--- a/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp
+++ b/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp
@@ -130,7 +130,7 @@ bool func_ternary_or(bool a, bool b, bool c, bool d, bool e, bool f) {
// CHECK: Branch,File 0, 122:31 -> 122:32 = (#4 - #5), #5 [3,0,2]
// CHECK: Branch,File 0, 122:36 -> 122:37 = (#2 - #3), #3 [2,0,0]
-bool func_if_nested_and_if(bool a, bool b, bool c, bool d, bool e) {
+bool func_if_nested_if(bool a, bool b, bool c, bool d, bool e) {
if (a || (b && c) || d || e)
return true;
else
@@ -144,7 +144,7 @@ bool func_if_nested_and_if(bool a, bool b, bool c, bool d, bool e) {
// CHECK: Branch,File 0, 134:24 -> 134:25 = (#4 - #5), #5 [3,0,2]
// CHECK: Branch,File 0, 134:29 -> 134:30 = (#2 - #3), #3 [2,0,0]
-bool func_ternary_nested_and_if(bool a, bool b, bool c, bool d, bool e) {
+bool func_ternary_nested_if(bool a, bool b, bool c, bool d, bool e) {
return (a || (b && c) || d || e) ? true : false;
}
@@ -155,7 +155,7 @@ bool func_ternary_nested_and_if(bool a, bool b, bool c, bool d, bool e) {
// CHECK: Branch,File 0, 148:28 -> 148:29 = (#4 - #5), #5 [3,0,2]
// CHECK: Branch,File 0, 148:33 -> 148:34 = (#2 - #3), #3 [2,0,0]
-bool func_if_nested_and_if_2(bool a, bool b, bool c, bool d, bool e) {
+bool func_if_nested_if_2(bool a, bool b, bool c, bool d, bool e) {
if (a || ((b && c) || d) && e)
return true;
else
@@ -169,7 +169,7 @@ bool func_if_nested_and_if_2(bool a, bool b, bool c, bool d, bool e) {
// CHECK: Branch,File 0, 159:25 -> 159:26 = (#5 - #6), #6 [4,3,0]
// CHECK: Branch,File 0, 159:31 -> 159:32 = #4, (#3 - #4) [3,0,0]
-bool func_ternary_nested_and_if_2(bool a, bool b, bool c, bool d, bool e) {
+bool func_ternary_nested_if_2(bool a, bool b, bool c, bool d, bool e) {
return (a || ((b && c) || d) && e) ? true : false;
}
@@ -179,3 +179,30 @@ bool func_ternary_nested_and_if_2(bool a, bool b, bool c, bool d, bool e) {
// CHECK: Branch,File 0, 173:23 -> 173:24 = #8, (#7 - #8) [5,3,4]
// CHECK: Branch,File 0, 173:29 -> 173:30 = (#5 - #6), #6 [4,3,0]
// CHECK: Branch,File 0, 173:35 -> 173:36 = #4, (#3 - #4) [3,0,0]
+
+bool func_if_nested_if_3(bool a, bool b, bool c, bool d, bool e, bool f) {
+ if ((a && (b || c) || (d && e)) && f)
+ return true;
+ else
+ return false;
+}
+
+// CHECK-LABEL: Decision,File 0, 184:7 -> 184:39 = M:0, C:6
+// CHECK: Branch,File 0, 184:8 -> 184:9 = #5, (#0 - #5) [1,4,3]
+// CHECK: Branch,File 0, 184:14 -> 184:15 = (#5 - #6), #6 [4,2,5]
+// CHECK: Branch,File 0, 184:19 -> 184:20 = (#6 - #7), #7 [5,2,3]
+// CHECK: Branch,File 0, 184:26 -> 184:27 = #8, (#4 - #8) [3,6,0]
+// CHECK: Branch,File 0, 184:31 -> 184:32 = #9, (#8 - #9) [6,2,0]
+// CHECK: Branch,File 0, 184:38 -> 184:39 = #3, (#2 - #3) [2,0,0]
+
+bool func_ternary_nested_if_3(bool a, bool b, bool c, bool d, bool e, bool f) {
+ return ((a && (b || c) || (d && e)) && f) ? true : false;
+}
+
+// CHECK-LABEL: Decision,File 0, 199:11 -> 199:43 = M:0, C:6
+// CHECK: Branch,File 0, 199:12 -> 199:13 = #5, (#0 - #5) [1,4,3]
+// CHECK: Branch,File 0, 199:18 -> 199:19 = (#5 - #6), #6 [4,2,5]
+// CHECK: Branch,File 0, 199:23 -> 199:24 = (#6 - #7), #7 [5,2,3]
+// CHECK: Branch,File 0, 199:30 -> 199:31 = #8, (#4 - #8) [3,6,0]
+// CHECK: Branch,File 0, 199:35 -> 199:36 = #9, (#8 - #9) [6,2,0]
+// CHECK: Branch,File 0, 199:42 -> 199:43 = #3, (#2 - #3) [2,0,0]
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 367c884957ff6d7..2a857917136a841 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -295,12 +295,11 @@ struct CounterMappingRegion {
Kind(Kind) {}
CounterMappingRegion(MCDCParameters MCDCParams, unsigned FileID,
- unsigned ExpandedFileID, unsigned LineStart,
- unsigned ColumnStart, unsigned LineEnd,
- unsigned ColumnEnd, RegionKind Kind)
- : MCDCParams(MCDCParams), FileID(FileID), ExpandedFileID(ExpandedFileID),
- LineStart(LineStart), ColumnStart(ColumnStart), LineEnd(LineEnd),
- ColumnEnd(ColumnEnd), Kind(Kind) {}
+ unsigned LineStart, unsigned ColumnStart,
+ unsigned LineEnd, unsigned ColumnEnd, RegionKind Kind)
+ : MCDCParams(MCDCParams), FileID(FileID), LineStart(LineStart),
+ ColumnStart(ColumnStart), LineEnd(LineEnd), ColumnEnd(ColumnEnd),
+ Kind(Kind) {}
static CounterMappingRegion
makeRegion(Counter Count, unsigned FileID, unsigned LineStart,
@@ -354,7 +353,7 @@ struct CounterMappingRegion {
makeDecisionRegion(MCDCParameters MCDCParams, unsigned FileID,
unsigned LineStart, unsigned ColumnStart, unsigned LineEnd,
unsigned ColumnEnd) {
- return CounterMappingRegion(MCDCParams, FileID, 0, LineStart, ColumnStart,
+ return CounterMappingRegion(MCDCParams, FileID, LineStart, ColumnStart,
LineEnd, ColumnEnd, MCDCDecisionRegion);
}
More information about the cfe-commits
mailing list