Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MC/DC][Coverage] Loosen the limit of NumConds from 6 #82448

Merged
merged 48 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
d168e0c
Implement MCDCTVIdxBuilder and MCDCTestVectorBuilder (LLVM side)
chapuni Feb 4, 2024
35b19ea
Revert "Implement MCDCTVIdxBuilder and MCDCTestVectorBuilder (LLVM si…
chapuni Feb 6, 2024
8c777eb
[Coverage] MCDCRecordProcessor: Find `ExecVectors` directly
chapuni Feb 6, 2024
56042d3
Merge branch 'mcdc/xv' into HEAD
chapuni Feb 6, 2024
5432aec
Implement MCDCTVIdxBuilder (LLVM side)
chapuni Feb 4, 2024
3ee8a61
Update comments and assertions
chapuni Feb 6, 2024
2fd504a
Merge remote-tracking branch 'origin/main' into mcdc/tvidx
chapuni Feb 8, 2024
1f0f3fc
Reorganize TVIdxBuilder
chapuni Feb 12, 2024
06c0801
Merge remote-tracking branch 'origin/main' into mcdc/tvidx
chapuni Feb 13, 2024
aa5b2f5
Merge remote-tracking branch 'origin/main' into HEAD
chapuni Feb 15, 2024
e3de647
[CoverageMapping] Refactor `mcdc::TVIdxBuilder`
chapuni Feb 15, 2024
753d0ad
remove <functional>
chapuni Feb 15, 2024
17cbac7
Update comments.
chapuni Feb 21, 2024
1a4ffa7
Add unittest
chapuni Feb 21, 2024
b5ecfcc
[MC/DC][Coverage] Loosen the limit of NumConds from 6
chapuni Feb 15, 2024
0ffad9c
Update testcases
chapuni Feb 21, 2024
c96fd2c
Use llvm::sort
chapuni Feb 21, 2024
357a693
EXPECT_
chapuni Feb 21, 2024
83d104c
Merge branch 'mcdc/tvidx' into HEAD
chapuni Feb 21, 2024
14c795e
Hide NConds
chapuni Feb 21, 2024
b6c1174
Merge remote-tracking branch 'chapuni/main' into mcdc/tvidx
chapuni Feb 25, 2024
cf936f6
Merge branch 'users/chapuni/mcdc/tvidx' into HEAD
chapuni Feb 25, 2024
662bdd6
Reformat
chapuni Feb 25, 2024
ac16655
Merge remote-tracking branch 'origin/main' into mcdc/clangtvidx
chapuni Feb 26, 2024
90bf8e9
Clarify bool
chapuni Feb 26, 2024
9eb6951
Update comments
chapuni Feb 26, 2024
86d67c6
Use MutableArrayRef :)
chapuni Feb 26, 2024
01abca2
Merge remote-tracking branch 'origin/main' into mcdc/clangtvidx
chapuni Feb 27, 2024
183c706
Merge branch 'main' into mcdc/clangtvidx
chapuni Apr 19, 2024
dd6f8be
Clarify 3rd arg of mcdc.tvbitmap.update is unused
chapuni May 9, 2024
cdd5531
Modify 3rd arg of mcdc.parameters to bits
chapuni May 8, 2024
54e6044
isMCDCBranch: Remove assert
chapuni May 9, 2024
b1a7100
isMCDCDecision: Remove assert
chapuni May 9, 2024
39802c5
Make llvm-cov capable of clang-18's profdata
chapuni May 14, 2024
f54c64d
Fix unittest
chapuni May 15, 2024
a6f7eef
Introduce `-fmcdc-max-conditions` and `-fmcdc-max-test-vectors`
chapuni May 16, 2024
0dd7ead
Merge branch 'main' into HEAD
chapuni May 20, 2024
cf837ae
mcdc-system-headers.cpp: Update
chapuni May 20, 2024
9e14c2a
llvm-cov tests for old (Version11) files
chapuni May 20, 2024
04f18ae
Merge branch 'main' into mcdc/clangtvidx
chapuni May 27, 2024
3ff3eb7
Update CoverageMapping/mcdc-scratch-space.c
chapuni May 27, 2024
296f5c5
Don't create tvbitmap_update if the record is allocated but excluded.
chapuni May 27, 2024
722424b
Update mcdc-error-conditions.cpp
chapuni Jun 10, 2024
183bc52
Update documents
chapuni Jun 10, 2024
83088f1
mcdc.tvupdate doesn't depend on condupdate
chapuni Jun 10, 2024
40872f5
Update SourceBasedCodeCoverage.rst
chapuni Jun 11, 2024
be5b28b
SourceBasedCodeCoverage.rst: s/you/users/
chapuni Jun 11, 2024
450d86b
SourceBasedCodeCoverage.rst: Clarify to be excluded.
chapuni Jun 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Merge remote-tracking branch 'origin/main' into mcdc/clangtvidx
  • Loading branch information
chapuni committed Feb 26, 2024
commit ac166557eaa40e0da54cfac3dfd1e1518aa66ca7
4 changes: 2 additions & 2 deletions clang/lib/CodeGen/CodeGenPGO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,8 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
return true;
}

// Otherwise, allocate the Decision. (Not activated yet)
MCDCState.DecisionByStmt[BinOp].Active = false;
// Otherwise, allocate the Decision.
MCDCState.DecisionByStmt[BinOp].BitmapIdx = 0;
}
return true;
}
Expand Down
154 changes: 61 additions & 93 deletions clang/lib/CodeGen/CoverageMappingGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,6 @@ struct MCDCCoverageBuilder {
llvm::SmallVector<mcdc::ConditionIDs> DecisionStack;
MCDC::State &MCDCState;
const Stmt *DecisionStmt = nullptr;
llvm::DenseMap<mcdc::ConditionID, mcdc::ConditionIDs> cache;
mcdc::ConditionID NextID = 0;
bool NotMapped = false;

Expand Down Expand Up @@ -740,14 +739,6 @@ struct MCDCCoverageBuilder {
return I->second.ID;
}

void ccc(const Expr *CondExpr, mcdc::ConditionIDs IDs) {
auto ID = getCondID(CondExpr);
if (ID < 0)
return;
if (!cache.contains(ID))
cache[ID] = IDs;
}

/// Return the LHS Decision ([0,0] if not set).
const mcdc::ConditionIDs &back() const { return DecisionStack.back(); }

Expand All @@ -768,7 +759,6 @@ struct MCDCCoverageBuilder {
return;

if (NextID == 0) {
cache.clear();
DecisionStmt = E;
assert(MCDCState.DecisionByStmt.contains(E));
}
Expand Down Expand Up @@ -822,37 +812,9 @@ struct MCDCCoverageBuilder {
// Set number of conditions and reset.
unsigned TotalConds = NextID;

llvm::SmallVector<mcdc::ConditionIDs> R(cache.size());
for (auto [ID, Conds] : cache)
R[ID] = Conds;
mcdc::TVIdxBuilder xxx(R);
auto NumTVs = xxx.NumTestVectors;
assert(MCDCState.DecisionByStmt.contains(E));
auto MaxTVs = mcdc::TVIdxBuilder::HardMaxTVs;
if (NumTVs == MaxTVs) {
auto &Diag = CGM.getDiags();
unsigned DiagID =
Diag.getCustomDiagID(DiagnosticsEngine::Warning,
"unsupported MC/DC boolean expression; "
"number of test vectors (%0) exceeds max (%1). "
"Expression will not be covered");
Diag.Report(E->getBeginLoc(), DiagID) << NumTVs << MaxTVs;

MCDCState.DecisionByStmt[E].Active = false; // Mark to be erased
} else {
// Last pos
MCDCState.DecisionByStmt[E] = {
MCDCState.BitmapBits, // Idx
MCDCState.BitmapBits += NumTVs, // Tail
std::move(xxx.Indices), // Indices
true, // Active
};
}

// Reset ID back to beginning.
NextID = 0;

// Return the number even if the Decision will be erased.
return TotalConds;
}
};
Expand Down Expand Up @@ -909,10 +871,6 @@ struct CounterCoverageMappingBuilder
return Counter::getCounter(CounterMap[S]);
}

auto getBitmapIdx(const Stmt *S) {
return MCDCState.DecisionByStmt[S].BitmapTailPos;
}

/// Push a region onto the stack.
///
/// Returns the index on the stack where the region was pushed. This can be
Expand Down Expand Up @@ -2019,38 +1977,66 @@ struct CounterCoverageMappingBuilder
subtractCounters(ParentCount, TrueCount));
}

void RewindDecision(unsigned Since) {
#ifndef NDEBUG
llvm::DenseSet<mcdc::ConditionID> SeenIDs;
unsigned NConds = 0;
#endif

assert(Since <= SourceRegions.size());
auto I = SourceRegions.begin() + Since;
while (I != SourceRegions.end()) {
if (I->isMCDCDecision()) {
assert(I->getMCDCDecisionParams().BitmapIdx == 0 &&
"It should be valid");
#ifndef NDEBUG
assert(NConds == 0 && "Duplicate MCDCDecision");
NConds = I->getMCDCDecisionParams().NumConditions;
assert(NConds > 0 && "Malformed MCDCDecision");
#endif
I = SourceRegions.erase(I);
continue;
}
void createOrCancelDecision(const BinaryOperator *E, unsigned Since) {
unsigned NumConds = MCDCBuilder.getTotalConditionsAndReset(E);
if (NumConds == 0)
return;

if (I->isMCDCBranch()) {
assert(SeenIDs.insert(I->getMCDCBranchParams().ID).second &&
"Duplicate CondID");
I->resetMCDCParams();
llvm::SmallVector<mcdc::ConditionIDs> CondIDs(NumConds);
for (const auto &SR : ArrayRef(SourceRegions).slice(Since)) {
if (SR.isMCDCBranch()) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you use make_filter_range from STLExtras?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think make_filter_range is less suitable here.

auto [ID, Conds] = SR.getMCDCBranchParams();
CondIDs[ID] = Conds;
}
}

mcdc::TVIdxBuilder Builder(CondIDs);
auto NumTVs = Builder.NumTestVectors;
assert(MCDCState.DecisionByStmt.contains(E));
auto MaxTVs = mcdc::TVIdxBuilder::HardMaxTVs;

if (NumTVs >= MaxTVs) {
// NumTVs exceeds MaxTVs -- warn and cancel the Decision.
cancelDecision(E, Since, NumTVs, MaxTVs);
return;
}

++I;
// The state for CodeGenPGO
MCDCState.DecisionByStmt[E] = {
MCDCState.BitmapBits, // Top
std::move(Builder.Indices),
};

auto DecisionParams = mcdc::DecisionParameters{
MCDCState.BitmapBits += NumTVs, // Tail
NumConds,
};

// Create MCDC Decision Region.
createDecisionRegion(E, DecisionParams);
}

// Warn and cancel the Decision.
void cancelDecision(const BinaryOperator *E, unsigned Since, int NumTVs,
int MaxTVs) {
auto &Diag = CVM.getCodeGenModule().getDiags();
unsigned DiagID =
Diag.getCustomDiagID(DiagnosticsEngine::Warning,
"unsupported MC/DC boolean expression; "
"number of test vectors (%0) exceeds max (%1). "
"Expression will not be covered");
Diag.Report(E->getBeginLoc(), DiagID) << NumTVs << MaxTVs;

// Restore MCDCBranch to Branch.
for (auto I = SourceRegions.begin() + Since, E = SourceRegions.end();
I != E; ++I) {
assert(!I->isMCDCDecision() && "Decision shouldn't be seen here");
if (I->isMCDCBranch())
I->resetMCDCParams();
}

assert(NConds > 0 && "MCDCDecision wasn't found");
assert(SeenIDs.size() == NConds && "Unexpected number of MCDCBranch(es)");
// Tell CodeGenPGO not to instrument.
MCDCState.DecisionByStmt.erase(E);
}

void VisitBinLAnd(const BinaryOperator *E) {
Expand All @@ -2075,14 +2061,6 @@ struct CounterCoverageMappingBuilder
// Track RHS True/False Decision.
const auto DecisionRHS = MCDCBuilder.back();

MCDCBuilder.ccc(E->getLHS(), DecisionLHS);
MCDCBuilder.ccc(E->getRHS(), DecisionRHS);

// Create MCDC Decision Region if at top-level (root).
unsigned NumConds = 0;
if (IsRootNode && (NumConds = MCDCBuilder.getTotalConditionsAndReset(E)))
createDecisionRegion(E, getBitmapIdx(E), NumConds);

// Extract the RHS's Execution Counter.
Counter RHSExecCnt = getRegionCounter(E);

Expand All @@ -2100,10 +2078,9 @@ struct CounterCoverageMappingBuilder
createBranchRegion(E->getRHS(), RHSTrueCnt,
subtractCounters(RHSExecCnt, RHSTrueCnt), DecisionRHS);

if (IsRootNode && NumConds > 0 && !MCDCState.DecisionByStmt[E].Active) {
RewindDecision(SourceRegionsSince);
MCDCState.DecisionByStmt.erase(E);
}
// Create MCDC Decision Region if at top-level (root).
if (IsRootNode)
createOrCancelDecision(E, SourceRegionsSince);
}

// Determine whether the right side of OR operation need to be visited.
Expand Down Expand Up @@ -2138,14 +2115,6 @@ struct CounterCoverageMappingBuilder
// Track RHS True/False Decision.
const auto DecisionRHS = MCDCBuilder.back();

MCDCBuilder.ccc(E->getLHS(), DecisionLHS);
MCDCBuilder.ccc(E->getRHS(), DecisionRHS);

// Create MCDC Decision Region if at top-level (root).
unsigned NumConds = 0;
if (IsRootNode && (NumConds = MCDCBuilder.getTotalConditionsAndReset(E)))
createDecisionRegion(E, getBitmapIdx(E), NumConds);

// Extract the RHS's Execution Counter.
Counter RHSExecCnt = getRegionCounter(E);

Expand All @@ -2167,10 +2136,9 @@ struct CounterCoverageMappingBuilder
createBranchRegion(E->getRHS(), subtractCounters(RHSExecCnt, RHSFalseCnt),
RHSFalseCnt, DecisionRHS);

if (IsRootNode && NumConds > 0 && !MCDCState.DecisionByStmt[E].Active) {
RewindDecision(SourceRegionsSince);
MCDCState.DecisionByStmt.erase(E);
}
// Create MCDC Decision Region if at top-level (root).
if (IsRootNode)
createOrCancelDecision(E, SourceRegionsSince);
}

void VisitLambdaExpr(const LambdaExpr *LE) {
Expand Down
2 changes: 0 additions & 2 deletions clang/lib/CodeGen/MCDCState.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ struct State {

struct Decision {
unsigned BitmapIdx;
unsigned BitmapTailPos;
llvm::SmallVector<std::array<int, 2>> Indices;
bool Active; /// Set false to cancel
};

llvm::DenseMap<const Stmt *, Decision> DecisionByStmt;
Expand Down
Loading
You are viewing a condensed version of this merge commit. You can view the full changes here.