Skip to content

Commit 2fda236

Browse files
chapunichapuni
committed
Fill up comments and revise
* Introduce `MCDCDecisionRecorder`. * Sink `DecisionRecord` (was `DecisionRow`) into `MCDCDecisionRecorder` and make it private. * Rename identifiers. * Replace `Expansions` with `ExpandedFileIDs` * Seek `Decisions` in ascent order.
1 parent 425185c commit 2fda236

File tree

1 file changed

+182
-128
lines changed

1 file changed

+182
-128
lines changed

‎llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

Lines changed: 182 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -583,84 +583,169 @@ static unsigned getMaxBitmapSize(const CounterMappingContext &Ctx,
583583
return MaxBitmapID + (SizeInBits / CHAR_BIT);
584584
}
585585

586-
/// This holds the DecisionRegion and MCDCBranch(es) under it.
587-
/// Also traverses Expansion(s).
588-
struct DecisionRow {
589-
/// The subject
590-
const CounterMappingRegion *DecisionRegion;
591-
592-
/// They are reflected from `DecisionRegion` for convenience.
593-
LineColPair DecisionStartLoc;
594-
LineColPair DecisionEndLoc;
595-
596-
/// This is passed to `MCDCRecordProcessor`, so this should be compatible to
597-
/// `ArrayRef<const CounterMappingRegion *>`.
598-
SmallVector<const CounterMappingRegion *, 6> Branches;
599-
600-
/// Each `ID` in `Branches` should be unique.
601-
DenseSet<CounterMappingRegion::MCDCConditionID> IDs;
602-
603-
/// Relevant `Expansion`(s) should be caught to find expanded Branches.
604-
SmallVector<const CounterMappingRegion *> Expansions;
605-
606-
DecisionRow(const CounterMappingRegion &Decision)
607-
: DecisionRegion(&Decision), DecisionStartLoc(Decision.startLoc()),
608-
DecisionEndLoc(Decision.endLoc()) {}
609-
610-
/// Determine whether `R` is included in `DecisionRegion`.
611-
bool inDecisionRegion(const CounterMappingRegion &R) {
612-
return (R.FileID == DecisionRegion->FileID &&
613-
R.startLoc() >= DecisionStartLoc && R.endLoc() <= DecisionEndLoc);
614-
}
586+
/// Collect Decisions, Branchs, and Expansions and associate them.
587+
class MCDCDecisionRecorder {
615588

616-
/// Determin whether `R` is pointed by any of Expansions.
617-
bool inExpansions(const CounterMappingRegion &R) {
618-
return any_of(Expansions, [&](const auto &Expansion) {
619-
return (Expansion->ExpandedFileID == R.FileID);
620-
});
621-
}
589+
private:
590+
/// This holds the DecisionRegion and MCDCBranches under it.
591+
/// Also traverses Expansion(s).
592+
/// The Decision has the number of MCDCBranches and
593+
struct DecisionRecord {
594+
const CounterMappingRegion *DecisionRegion;
595+
596+
/// They are reflected from DecisionRegion for convenience.
597+
LineColPair DecisionStartLoc;
598+
LineColPair DecisionEndLoc;
599+
600+
/// This is passed to `MCDCRecordProcessor`, so this should be compatible
601+
/// to`ArrayRef<const CounterMappingRegion *>`.
602+
SmallVector<const CounterMappingRegion *> MCDCBranches;
603+
604+
/// IDs that are stored in MCDCBranches
605+
/// Complete when all IDs (1 to NumConditions) are met.
606+
DenseSet<CounterMappingRegion::MCDCConditionID> ConditionIDs;
607+
608+
/// Set of IDs of Expansion(s) that are relevant to DecisionRegion
609+
/// and its children (via expansions).
610+
/// FileID pointed by ExpandedFileID is dedicated to the expansion, so
611+
/// the location in the expansion doesn't matter.
612+
DenseSet<unsigned> ExpandedFileIDs;
613+
614+
DecisionRecord(const CounterMappingRegion &Decision)
615+
: DecisionRegion(&Decision), DecisionStartLoc(Decision.startLoc()),
616+
DecisionEndLoc(Decision.endLoc()) {
617+
assert(Decision.Kind == CounterMappingRegion::MCDCDecisionRegion);
618+
}
622619

623-
enum class UpdateResult {
624-
NotFound = 0,
625-
Updated,
626-
Completed,
627-
};
620+
/// Determine whether DecisionRecord dominates `R`.
621+
bool dominates(const CounterMappingRegion &R) {
622+
// Determine whether `R` is included in `DecisionRegion`.
623+
if (R.FileID == DecisionRegion->FileID &&
624+
R.startLoc() >= DecisionStartLoc && R.endLoc() <= DecisionEndLoc)
625+
return true;
626+
627+
// Determine whether `R` is pointed by any of Expansions.
628+
return ExpandedFileIDs.contains(R.FileID);
629+
}
630+
631+
enum Result {
632+
NotProcessed = 0, /// Irrelevant to this Decision
633+
Processed, /// Added to this Decision
634+
Completed, /// Added and filled this Decision
635+
};
628636

629-
/// Add `Branch` into the Decision
630-
UpdateResult updateBranch(const CounterMappingRegion &Branch) {
631-
auto ID = Branch.MCDCParams.ID;
632-
assert(ID > 0 && "MCDCBranch.ID should begin with 1");
637+
/// Add Branch into the Decision
638+
/// \param Branch expects MCDCBranchRegion
639+
/// \returns NotProcessed/Processed/Completed
640+
Result addBranch(const CounterMappingRegion &Branch) {
641+
assert(Branch.Kind == CounterMappingRegion::MCDCBranchRegion);
633642

634-
if (!IDs.contains(ID) &&
635-
(inDecisionRegion(Branch) || inExpansions(Branch))) {
636-
assert(Branches.size() < DecisionRegion->MCDCParams.NumConditions);
643+
auto ConditionID = Branch.MCDCParams.ID;
644+
assert(ConditionID > 0 && "ConditionID should begin with 1");
637645

638-
// Put `ID=1` in front of `Branches` for convenience
639-
// even if `Branches` is not topological.
640-
if (ID == 1)
641-
Branches.insert(Branches.begin(), &Branch);
646+
if (ConditionIDs.contains(ConditionID) ||
647+
ConditionID > DecisionRegion->MCDCParams.NumConditions)
648+
return NotProcessed;
649+
650+
if (!this->dominates(Branch))
651+
return NotProcessed;
652+
653+
assert(MCDCBranches.size() < DecisionRegion->MCDCParams.NumConditions);
654+
655+
// Put `ID=1` in front of `MCDCBranches` for convenience
656+
// even if `MCDCBranches` is not topological.
657+
if (ConditionID == 1)
658+
MCDCBranches.insert(MCDCBranches.begin(), &Branch);
642659
else
643-
Branches.push_back(&Branch);
660+
MCDCBranches.push_back(&Branch);
644661

645662
// Mark `ID` as `assigned`.
646-
IDs.insert(ID);
647-
648-
// `Completed` when `Branches` is full
649-
return (Branches.size() == DecisionRegion->MCDCParams.NumConditions
650-
? UpdateResult::Completed
651-
: UpdateResult::Updated);
652-
} else
653-
return UpdateResult::NotFound;
654-
}
663+
ConditionIDs.insert(ConditionID);
664+
665+
// `Completed` when `MCDCBranches` is full
666+
return (MCDCBranches.size() == DecisionRegion->MCDCParams.NumConditions
667+
? Completed
668+
: Processed);
669+
}
670+
671+
/// Record Expansion if it is relevant to this Decision.
672+
/// Each `Expansion` may nest.
673+
/// \returns true if recorded.
674+
bool recordExpansion(const CounterMappingRegion &Expansion) {
675+
if (!this->dominates(Expansion))
676+
return false;
655677

656-
/// Record `Expansion` if it is dominated to the Decision.
657-
/// Each `Expansion` may nest.
658-
bool updateExpansion(const CounterMappingRegion &Expansion) {
659-
if (inDecisionRegion(Expansion) || inExpansions(Expansion)) {
660-
Expansions.push_back(&Expansion);
678+
ExpandedFileIDs.insert(Expansion.ExpandedFileID);
661679
return true;
662-
} else
680+
}
681+
};
682+
683+
private:
684+
/// Decisions in progress
685+
/// DecisionRecord is added for each MCDCDecisionRegion.
686+
/// DecisionRecord is removed when Decision is completed.
687+
SmallVector<DecisionRecord> Decisions;
688+
689+
public:
690+
~MCDCDecisionRecorder() {
691+
assert(Decisions.empty() && "All Decisions have not been resolved");
692+
}
693+
694+
/// Register Region and start recording if it is MCDCDecisionRegion.
695+
/// \param Region to be inspected
696+
/// \returns true if recording started.
697+
bool registerDecision(const CounterMappingRegion &Region) {
698+
if (Region.Kind != CounterMappingRegion::MCDCDecisionRegion)
663699
return false;
700+
701+
// Start recording Region to create DecisionRecord
702+
Decisions.emplace_back(Region);
703+
return true;
704+
}
705+
706+
using DecisionAndBranches =
707+
std::pair<const CounterMappingRegion *, /// Decision
708+
SmallVector<const CounterMappingRegion *> /// Branches
709+
>;
710+
711+
/// If Region is ExpansionRegion, record it.
712+
/// If Region is MCDCBranchRegion, add it to DecisionRecord.
713+
/// \param Region to be inspected
714+
/// \returns DecisionsAndBranches if DecisionRecord completed.
715+
/// Or returns nullopt.
716+
std::optional<DecisionAndBranches>
717+
processRegion(const CounterMappingRegion &Region) {
718+
719+
// Record ExpansionRegion.
720+
if (Region.Kind == CounterMappingRegion::ExpansionRegion) {
721+
for (auto &Decision : reverse(Decisions)) {
722+
if (Decision.recordExpansion(Region))
723+
break;
724+
}
725+
return std::nullopt; // It doesn't complete.
726+
}
727+
728+
// Do nothing unless MCDCBranchRegion.
729+
if (Region.Kind != CounterMappingRegion::MCDCBranchRegion)
730+
return std::nullopt;
731+
732+
// Seek each Decision and apply Region to it.
733+
for (auto DecisionIter = Decisions.begin(), DecisionEnd = Decisions.end();
734+
DecisionIter != DecisionEnd; ++DecisionIter)
735+
switch (DecisionIter->addBranch(Region)) {
736+
case DecisionRecord::NotProcessed:
737+
continue;
738+
case DecisionRecord::Processed:
739+
return std::nullopt;
740+
case DecisionRecord::Completed:
741+
DecisionAndBranches Result =
742+
std::make_pair(DecisionIter->DecisionRegion,
743+
std::move(DecisionIter->MCDCBranches));
744+
Decisions.erase(DecisionIter); // No longer used.
745+
return Result;
746+
}
747+
748+
llvm_unreachable("Branch not found in Decisions");
664749
}
665750
};
666751

@@ -720,14 +805,13 @@ Error CoverageMapping::loadFunctionRecord(
720805
Record.MappingRegions[0].Count.isZero() && Counts[0] > 0)
721806
return Error::success();
722807

723-
SmallVector<DecisionRow> Decisions;
808+
MCDCDecisionRecorder MCDCDecisions;
724809
FunctionRecord Function(OrigFuncName, Record.Filenames);
725810
for (const auto &Region : Record.MappingRegions) {
726-
if (Region.Kind == CounterMappingRegion::MCDCDecisionRegion) {
727-
// Start recording `Region` as the `Decision`
728-
Decisions.emplace_back(Region);
811+
// MCDCDecisionRegion should be handled first since it overlaps with
812+
// others inside.
813+
if (MCDCDecisions.registerDecision(Region))
729814
continue;
730-
}
731815
Expected<int64_t> ExecutionCount = Ctx.evaluate(Region.Count);
732816
if (auto E = ExecutionCount.takeError()) {
733817
consumeError(std::move(E));
@@ -740,69 +824,39 @@ Error CoverageMapping::loadFunctionRecord(
740824
}
741825
Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount);
742826

743-
if (Region.Kind == CounterMappingRegion::ExpansionRegion) {
744-
for (auto &Decision : reverse(Decisions)) {
745-
if (Decision.updateExpansion(Region))
746-
break;
747-
}
827+
auto Result = MCDCDecisions.processRegion(Region);
828+
if (!Result) // Any Decision doesn't complete.
748829
continue;
749-
}
750830

751-
if (Region.Kind != CounterMappingRegion::MCDCBranchRegion)
752-
continue;
753-
754-
// If a MCDCDecisionRegion was seen, store the BranchRegions that
755-
// correspond to it in a vector, according to the number of conditions
756-
// recorded for the region (tracked by NumConds).
757-
for (int I = Decisions.size() - 1; I >= 0; --I) {
758-
auto &Decision = Decisions[I];
759-
760-
// As we move through all of the MCDCBranchRegions that follow the
761-
// MCDCDecisionRegion, decrement NumConds to make sure we account for
762-
// them all before we calculate the bitmap of executed test vectors.
763-
switch (Decision.updateBranch(Region)) {
764-
case DecisionRow::UpdateResult::NotFound:
765-
continue;
766-
case DecisionRow::UpdateResult::Updated:
767-
goto branch_found;
768-
case DecisionRow::UpdateResult::Completed:
769-
// Evaluating the test vector bitmap for the decision region entails
770-
// calculating precisely what bits are pertinent to this region alone.
771-
// This is calculated based on the recorded offset into the global
772-
// profile bitmap; the length is calculated based on the recorded
773-
// number of conditions.
774-
Expected<BitVector> ExecutedTestVectorBitmap =
775-
Ctx.evaluateBitmap(Decision.DecisionRegion);
776-
if (auto E = ExecutedTestVectorBitmap.takeError()) {
777-
consumeError(std::move(E));
778-
return Error::success();
779-
}
780-
781-
// Since the bitmap identifies the executed test vectors for an MC/DC
782-
// DecisionRegion, all of the information is now available to process.
783-
// This is where the bulk of the MC/DC progressing takes place.
784-
Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
785-
*Decision.DecisionRegion, *ExecutedTestVectorBitmap,
786-
Decision.Branches);
787-
if (auto E = Record.takeError()) {
788-
consumeError(std::move(E));
789-
return Error::success();
790-
}
791-
792-
// Save the MC/DC Record so that it can be visualized later.
793-
Function.pushMCDCRecord(*Record);
831+
auto MCDCDecision = Result->first;
832+
auto &MCDCBranches = Result->second;
833+
834+
// Evaluating the test vector bitmap for the decision region entails
835+
// calculating precisely what bits are pertinent to this region alone.
836+
// This is calculated based on the recorded offset into the global
837+
// profile bitmap; the length is calculated based on the recorded
838+
// number of conditions.
839+
Expected<BitVector> ExecutedTestVectorBitmap =
840+
Ctx.evaluateBitmap(MCDCDecision);
841+
if (auto E = ExecutedTestVectorBitmap.takeError()) {
842+
consumeError(std::move(E));
843+
return Error::success();
844+
}
794845

795-
Decisions.erase(Decisions.begin() + I);
796-
goto branch_found;
797-
}
846+
// Since the bitmap identifies the executed test vectors for an MC/DC
847+
// DecisionRegion, all of the information is now available to process.
848+
// This is where the bulk of the MC/DC progressing takes place.
849+
Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
850+
*MCDCDecision, *ExecutedTestVectorBitmap, MCDCBranches);
851+
if (auto E = Record.takeError()) {
852+
consumeError(std::move(E));
853+
return Error::success();
798854
}
799-
llvm_unreachable("Branch not found in Decisions");
800855

801-
branch_found:;
856+
// Save the MC/DC Record so that it can be visualized later.
857+
Function.pushMCDCRecord(*Record);
802858
}
803859

804-
assert(Decisions.empty() && "All Decisions have not been resolved");
805-
806860
// Don't create records for (filenames, function) pairs we've already seen.
807861
auto FilenamesHash = hash_combine_range(Record.Filenames.begin(),
808862
Record.Filenames.end());

0 commit comments

Comments
 (0)