Skip to content

Commit b3306cb

Browse files
authored
[DFAJumpThreading] Prevent pass from using too much memory. (#153193)
The limit 'dfa-max-num-paths' that is used to control number of enumerated paths was not checked against inside getPathsFromStateDefMap. It may lead to large memory consumption for complex enough switch statements. Reland #145482
1 parent b40d233 commit b3306cb

File tree

1 file changed

+25
-15
lines changed

1 file changed

+25
-15
lines changed

llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -581,15 +581,17 @@ struct AllSwitchPaths {
581581
VisitedBlocks VB;
582582
// Get paths from the determinator BBs to SwitchPhiDefBB
583583
std::vector<ThreadingPath> PathsToPhiDef =
584-
getPathsFromStateDefMap(StateDef, SwitchPhi, VB);
585-
if (SwitchPhiDefBB == SwitchBlock) {
584+
getPathsFromStateDefMap(StateDef, SwitchPhi, VB, MaxNumPaths);
585+
if (SwitchPhiDefBB == SwitchBlock || PathsToPhiDef.empty()) {
586586
TPaths = std::move(PathsToPhiDef);
587587
return;
588588
}
589589

590+
assert(MaxNumPaths >= PathsToPhiDef.size() && !PathsToPhiDef.empty());
591+
auto PathsLimit = MaxNumPaths / PathsToPhiDef.size();
590592
// Find and append paths from SwitchPhiDefBB to SwitchBlock.
591593
PathsType PathsToSwitchBB =
592-
paths(SwitchPhiDefBB, SwitchBlock, VB, /* PathDepth = */ 1);
594+
paths(SwitchPhiDefBB, SwitchBlock, VB, /* PathDepth = */ 1, PathsLimit);
593595
if (PathsToSwitchBB.empty())
594596
return;
595597

@@ -610,13 +612,16 @@ struct AllSwitchPaths {
610612
typedef DenseMap<const BasicBlock *, const PHINode *> StateDefMap;
611613
std::vector<ThreadingPath> getPathsFromStateDefMap(StateDefMap &StateDef,
612614
PHINode *Phi,
613-
VisitedBlocks &VB) {
615+
VisitedBlocks &VB,
616+
unsigned PathsLimit) {
614617
std::vector<ThreadingPath> Res;
615618
auto *PhiBB = Phi->getParent();
616619
VB.insert(PhiBB);
617620

618621
VisitedBlocks UniqueBlocks;
619622
for (auto *IncomingBB : Phi->blocks()) {
623+
if (Res.size() >= PathsLimit)
624+
break;
620625
if (!UniqueBlocks.insert(IncomingBB).second)
621626
continue;
622627
if (!SwitchOuterLoop->contains(IncomingBB))
@@ -652,8 +657,9 @@ struct AllSwitchPaths {
652657

653658
// Direct predecessor, just add to the path.
654659
if (IncomingPhiDefBB == IncomingBB) {
655-
std::vector<ThreadingPath> PredPaths =
656-
getPathsFromStateDefMap(StateDef, IncomingPhi, VB);
660+
assert(PathsLimit > Res.size());
661+
std::vector<ThreadingPath> PredPaths = getPathsFromStateDefMap(
662+
StateDef, IncomingPhi, VB, PathsLimit - Res.size());
657663
for (ThreadingPath &Path : PredPaths) {
658664
Path.push_back(PhiBB);
659665
Res.push_back(std::move(Path));
@@ -666,13 +672,17 @@ struct AllSwitchPaths {
666672
continue;
667673

668674
PathsType IntermediatePaths;
669-
IntermediatePaths =
670-
paths(IncomingPhiDefBB, IncomingBB, VB, /* PathDepth = */ 1);
675+
assert(PathsLimit > Res.size());
676+
auto InterPathLimit = PathsLimit - Res.size();
677+
IntermediatePaths = paths(IncomingPhiDefBB, IncomingBB, VB,
678+
/* PathDepth = */ 1, InterPathLimit);
671679
if (IntermediatePaths.empty())
672680
continue;
673681

682+
assert(InterPathLimit >= IntermediatePaths.size());
683+
auto PredPathLimit = InterPathLimit / IntermediatePaths.size();
674684
std::vector<ThreadingPath> PredPaths =
675-
getPathsFromStateDefMap(StateDef, IncomingPhi, VB);
685+
getPathsFromStateDefMap(StateDef, IncomingPhi, VB, PredPathLimit);
676686
for (const ThreadingPath &Path : PredPaths) {
677687
for (const PathType &IPath : IntermediatePaths) {
678688
ThreadingPath NewPath(Path);
@@ -687,7 +697,7 @@ struct AllSwitchPaths {
687697
}
688698

689699
PathsType paths(BasicBlock *BB, BasicBlock *ToBB, VisitedBlocks &Visited,
690-
unsigned PathDepth) {
700+
unsigned PathDepth, unsigned PathsLimit) {
691701
PathsType Res;
692702

693703
// Stop exploring paths after visiting MaxPathLength blocks
@@ -714,6 +724,8 @@ struct AllSwitchPaths {
714724
// is used to prevent a duplicate path from being generated
715725
SmallPtrSet<BasicBlock *, 4> Successors;
716726
for (BasicBlock *Succ : successors(BB)) {
727+
if (Res.size() >= PathsLimit)
728+
break;
717729
if (!Successors.insert(Succ).second)
718730
continue;
719731

@@ -735,14 +747,12 @@ struct AllSwitchPaths {
735747
// coverage and compile time.
736748
if (LI->getLoopFor(Succ) != CurrLoop)
737749
continue;
738-
739-
PathsType SuccPaths = paths(Succ, ToBB, Visited, PathDepth + 1);
750+
assert(PathsLimit > Res.size());
751+
PathsType SuccPaths =
752+
paths(Succ, ToBB, Visited, PathDepth + 1, PathsLimit - Res.size());
740753
for (PathType &Path : SuccPaths) {
741754
Path.push_front(BB);
742755
Res.push_back(Path);
743-
if (Res.size() >= MaxNumPaths) {
744-
return Res;
745-
}
746756
}
747757
}
748758
// This block could now be visited again from a different predecessor. Note

0 commit comments

Comments
 (0)