aboutsummaryrefslogtreecommitdiff
path: root/clang/lib
diff options
context:
space:
mode:
authorHans Wennborg <hans@chromium.org>2024-06-14 10:32:39 +0200
committerHans Wennborg <hans@chromium.org>2024-06-14 10:47:41 +0200
commitb422fa6b62160f5eeb038d816d05e039182dde56 (patch)
tree1facf1d549693b6e685af22a7eebbe8b4db9daa4 /clang/lib
parent880d37038c7bbff53ef02c9d6b01cbbc87875243 (diff)
downloadllvm-b422fa6b62160f5eeb038d816d05e039182dde56.zip
llvm-b422fa6b62160f5eeb038d816d05e039182dde56.tar.gz
llvm-b422fa6b62160f5eeb038d816d05e039182dde56.tar.bz2
Revert "[MC/DC][Coverage] Loosen the limit of NumConds from 6 (#82448)"
This broke the lit tests on Mac: https://green.lab.llvm.org/job/llvm.org/job/clang-stage1-RA/1096/ > By storing possible test vectors instead of combinations of conditions, > the restriction is dramatically relaxed. > > This introduces two options to `cc1`: > > * `-fmcdc-max-conditions=32767` > * `-fmcdc-max-test-vectors=2147483646` > > This change makes coverage mapping, profraw, and profdata incompatible > with Clang-18. > > - Bitmap semantics changed. It is incompatible with previous format. > - `BitmapIdx` in `Decision` points to the end of the bitmap. > - Bitmap is packed per function. > - `llvm-cov` can understand `profdata` generated by `llvm-profdata-18`. > > RFC: > https://discourse.llvm.org/t/rfc-coverage-new-algorithm-and-file-format-for-mc-dc/76798 This reverts commit 7ead2d8c7e9114b3f23666209a1654939987cb30.
Diffstat (limited to 'clang/lib')
-rw-r--r--clang/lib/CodeGen/CodeGenPGO.cpp50
-rw-r--r--clang/lib/CodeGen/CoverageMappingGen.cpp77
-rw-r--r--clang/lib/CodeGen/MCDCState.h4
3 files changed, 29 insertions, 102 deletions
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 59139e3..db8e6f5 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -167,6 +167,8 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
PGOHash Hash;
/// The map of statements to counters.
llvm::DenseMap<const Stmt *, unsigned> &CounterMap;
+ /// The next bitmap byte index to assign.
+ unsigned NextMCDCBitmapIdx;
/// The state of MC/DC Coverage in this function.
MCDC::State &MCDCState;
/// Maximum number of supported MC/DC conditions in a boolean expression.
@@ -181,7 +183,7 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
MCDC::State &MCDCState, unsigned MCDCMaxCond,
DiagnosticsEngine &Diag)
: NextCounter(0), Hash(HashVersion), CounterMap(CounterMap),
- MCDCState(MCDCState), MCDCMaxCond(MCDCMaxCond),
+ NextMCDCBitmapIdx(0), MCDCState(MCDCState), MCDCMaxCond(MCDCMaxCond),
ProfileVersion(ProfileVersion), Diag(Diag) {}
// Blocks and lambdas are handled as separate functions, so we need not
@@ -312,8 +314,11 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
return true;
}
- // Otherwise, allocate the Decision.
- MCDCState.DecisionByStmt[BinOp].BitmapIdx = 0;
+ // Otherwise, allocate the number of bytes required for the bitmap
+ // based on the number of conditions. Must be at least 1-byte long.
+ MCDCState.DecisionByStmt[BinOp].BitmapIdx = NextMCDCBitmapIdx;
+ unsigned SizeInBits = std::max<unsigned>(1L << NumCond, CHAR_BIT);
+ NextMCDCBitmapIdx += SizeInBits / CHAR_BIT;
}
return true;
}
@@ -1078,9 +1083,7 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) {
// for most embedded applications. Setting a maximum value prevents the
// bitmap footprint from growing too large without the user's knowledge. In
// the future, this value could be adjusted with a command-line option.
- unsigned MCDCMaxConditions =
- (CGM.getCodeGenOpts().MCDCCoverage ? CGM.getCodeGenOpts().MCDCMaxConds
- : 0);
+ unsigned MCDCMaxConditions = (CGM.getCodeGenOpts().MCDCCoverage) ? 6 : 0;
RegionCounterMap.reset(new llvm::DenseMap<const Stmt *, unsigned>);
RegionMCDCState.reset(new MCDC::State);
@@ -1096,6 +1099,7 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) {
Walker.TraverseDecl(const_cast<CapturedDecl *>(CD));
assert(Walker.NextCounter > 0 && "no entry counter mapped for decl");
NumRegionCounters = Walker.NextCounter;
+ RegionMCDCState->BitmapBytes = Walker.NextMCDCBitmapIdx;
FunctionHash = Walker.Hash.finalize();
}
@@ -1228,7 +1232,7 @@ void CodeGenPGO::emitMCDCParameters(CGBuilderTy &Builder) {
// anything.
llvm::Value *Args[3] = {llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy),
Builder.getInt64(FunctionHash),
- Builder.getInt32(RegionMCDCState->BitmapBits)};
+ Builder.getInt32(RegionMCDCState->BitmapBytes)};
Builder.CreateCall(
CGM.getIntrinsic(llvm::Intrinsic::instrprof_mcdc_parameters), Args);
}
@@ -1246,11 +1250,6 @@ void CodeGenPGO::emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder,
if (DecisionStateIter == RegionMCDCState->DecisionByStmt.end())
return;
- // Don't create tvbitmap_update if the record is allocated but excluded.
- // Or `bitmap |= (1 << 0)` would be wrongly executed to the next bitmap.
- if (DecisionStateIter->second.Indices.size() == 0)
- return;
-
// Extract the offset of the global bitmap associated with this expression.
unsigned MCDCTestVectorBitmapOffset = DecisionStateIter->second.BitmapIdx;
auto *I8PtrTy = llvm::PointerType::getUnqual(CGM.getLLVMContext());
@@ -1262,7 +1261,7 @@ void CodeGenPGO::emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder,
// index represents an executed test vector.
llvm::Value *Args[5] = {llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy),
Builder.getInt64(FunctionHash),
- Builder.getInt32(0), // Unused
+ Builder.getInt32(RegionMCDCState->BitmapBytes),
Builder.getInt32(MCDCTestVectorBitmapOffset),
MCDCCondBitmapAddr.emitRawPointer(CGF)};
Builder.CreateCall(
@@ -1306,22 +1305,19 @@ void CodeGenPGO::emitMCDCCondBitmapUpdate(CGBuilderTy &Builder, const Expr *S,
// Extract the ID of the condition we are setting in the bitmap.
const auto &Branch = BranchStateIter->second;
assert(Branch.ID >= 0 && "Condition has no ID!");
- assert(Branch.DecisionStmt);
-
- // Cancel the emission if the Decision is erased after the allocation.
- const auto DecisionIter =
- RegionMCDCState->DecisionByStmt.find(Branch.DecisionStmt);
- if (DecisionIter == RegionMCDCState->DecisionByStmt.end())
- return;
- const auto &TVIdxs = DecisionIter->second.Indices[Branch.ID];
+ auto *I8PtrTy = llvm::PointerType::getUnqual(CGM.getLLVMContext());
- auto *CurTV = Builder.CreateLoad(MCDCCondBitmapAddr,
- "mcdc." + Twine(Branch.ID + 1) + ".cur");
- auto *NewTV = Builder.CreateAdd(CurTV, Builder.getInt32(TVIdxs[true]));
- NewTV = Builder.CreateSelect(
- Val, NewTV, Builder.CreateAdd(CurTV, Builder.getInt32(TVIdxs[false])));
- Builder.CreateStore(NewTV, MCDCCondBitmapAddr);
+ // Emit intrinsic that updates a dedicated temporary value on the stack after
+ // a condition is evaluated. After the set of conditions has been updated,
+ // the resulting value is used to update the boolean expression's bitmap.
+ llvm::Value *Args[5] = {llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy),
+ Builder.getInt64(FunctionHash),
+ Builder.getInt32(Branch.ID),
+ MCDCCondBitmapAddr.emitRawPointer(CGF), Val};
+ Builder.CreateCall(
+ CGM.getIntrinsic(llvm::Intrinsic::instrprof_mcdc_condbitmap_update),
+ Args);
}
void CodeGenPGO::setValueProfilingFlag(llvm::Module &M) {
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index ba483d8..6ce2d32 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -195,10 +195,6 @@ public:
return std::holds_alternative<mcdc::BranchParameters>(MCDCParams);
}
- const auto &getMCDCBranchParams() const {
- return mcdc::getParams<const mcdc::BranchParameters>(MCDCParams);
- }
-
bool isMCDCDecision() const {
return std::holds_alternative<mcdc::DecisionParameters>(MCDCParams);
}
@@ -208,8 +204,6 @@ public:
}
const mcdc::Parameters &getMCDCParams() const { return MCDCParams; }
-
- void resetMCDCParams() { MCDCParams = mcdc::Parameters(); }
};
/// Spelling locations for the start and end of a source region.
@@ -754,7 +748,6 @@ private:
llvm::SmallVector<mcdc::ConditionIDs> DecisionStack;
MCDC::State &MCDCState;
- const Stmt *DecisionStmt = nullptr;
mcdc::ConditionID NextID = 0;
bool NotMapped = false;
@@ -784,8 +777,7 @@ public:
/// Set the given condition's ID.
void setCondID(const Expr *Cond, mcdc::ConditionID ID) {
- MCDCState.BranchByStmt[CodeGenFunction::stripCond(Cond)] = {ID,
- DecisionStmt};
+ MCDCState.BranchByStmt[CodeGenFunction::stripCond(Cond)].ID = ID;
}
/// Return the ID of a given condition.
@@ -816,11 +808,6 @@ public:
if (NotMapped)
return;
- if (NextID == 0) {
- DecisionStmt = E;
- assert(MCDCState.DecisionByStmt.contains(E));
- }
-
const mcdc::ConditionIDs &ParentDecision = DecisionStack.back();
// If the operator itself has an assigned ID, this means it represents a
@@ -2135,41 +2122,13 @@ struct CounterCoverageMappingBuilder
subtractCounters(ParentCount, TrueCount));
}
- void createOrCancelDecision(const BinaryOperator *E, unsigned Since) {
+ void createDecision(const BinaryOperator *E) {
unsigned NumConds = MCDCBuilder.getTotalConditionsAndReset(E);
if (NumConds == 0)
return;
- // Extract [ID, Conds] to construct the graph.
- llvm::SmallVector<mcdc::ConditionIDs> CondIDs(NumConds);
- for (const auto &SR : ArrayRef(SourceRegions).slice(Since)) {
- if (SR.isMCDCBranch()) {
- auto [ID, Conds] = SR.getMCDCBranchParams();
- CondIDs[ID] = Conds;
- }
- }
-
- // Construct the graph and calculate `Indices`.
- mcdc::TVIdxBuilder Builder(CondIDs);
- unsigned NumTVs = Builder.NumTestVectors;
- unsigned MaxTVs = CVM.getCodeGenModule().getCodeGenOpts().MCDCMaxTVs;
- assert(MaxTVs < mcdc::TVIdxBuilder::HardMaxTVs);
-
- if (NumTVs > MaxTVs) {
- // NumTVs exceeds MaxTVs -- warn and cancel the Decision.
- cancelDecision(E, Since, NumTVs, MaxTVs);
- return;
- }
-
- // Update the state for CodeGenPGO
- assert(MCDCState.DecisionByStmt.contains(E));
- MCDCState.DecisionByStmt[E] = {
- MCDCState.BitmapBits, // Top
- std::move(Builder.Indices),
- };
-
auto DecisionParams = mcdc::DecisionParameters{
- MCDCState.BitmapBits += NumTVs, // Tail
+ MCDCState.DecisionByStmt[E].BitmapIdx,
NumConds,
};
@@ -2177,28 +2136,6 @@ struct CounterCoverageMappingBuilder
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 &SR : MutableArrayRef(SourceRegions).slice(Since)) {
- assert(!SR.isMCDCDecision() && "Decision shouldn't be seen here");
- if (SR.isMCDCBranch())
- SR.resetMCDCParams();
- }
-
- // Tell CodeGenPGO not to instrument.
- MCDCState.DecisionByStmt.erase(E);
- }
-
/// Check if E belongs to system headers.
bool isExprInSystemHeader(const BinaryOperator *E) const {
return (!SystemHeadersCoverage &&
@@ -2215,8 +2152,6 @@ struct CounterCoverageMappingBuilder
bool IsRootNode = MCDCBuilder.isIdle();
- unsigned SourceRegionsSince = SourceRegions.size();
-
// Keep track of Binary Operator and assign MCDC condition IDs.
MCDCBuilder.pushAndAssignIDs(E);
@@ -2255,7 +2190,7 @@ struct CounterCoverageMappingBuilder
// Create MCDC Decision Region if at top-level (root).
if (IsRootNode)
- createOrCancelDecision(E, SourceRegionsSince);
+ createDecision(E);
}
// Determine whether the right side of OR operation need to be visited.
@@ -2276,8 +2211,6 @@ struct CounterCoverageMappingBuilder
bool IsRootNode = MCDCBuilder.isIdle();
- unsigned SourceRegionsSince = SourceRegions.size();
-
// Keep track of Binary Operator and assign MCDC condition IDs.
MCDCBuilder.pushAndAssignIDs(E);
@@ -2320,7 +2253,7 @@ struct CounterCoverageMappingBuilder
// Create MCDC Decision Region if at top-level (root).
if (IsRootNode)
- createOrCancelDecision(E, SourceRegionsSince);
+ createDecision(E);
}
void VisitLambdaExpr(const LambdaExpr *LE) {
diff --git a/clang/lib/CodeGen/MCDCState.h b/clang/lib/CodeGen/MCDCState.h
index e0dd28f..29b6f0f 100644
--- a/clang/lib/CodeGen/MCDCState.h
+++ b/clang/lib/CodeGen/MCDCState.h
@@ -27,18 +27,16 @@ using namespace llvm::coverage::mcdc;
/// Per-Function MC/DC state
struct State {
- unsigned BitmapBits = 0;
+ unsigned BitmapBytes = 0;
struct Decision {
unsigned BitmapIdx;
- llvm::SmallVector<std::array<int, 2>> Indices;
};
llvm::DenseMap<const Stmt *, Decision> DecisionByStmt;
struct Branch {
ConditionID ID;
- const Stmt *DecisionStmt;
};
llvm::DenseMap<const Stmt *, Branch> BranchByStmt;