Skip to content

Commit eba02ff

Browse files
paperchalicepaperchalice
committed
Address comments
1 parent f00a60a commit eba02ff

File tree

2 files changed

+71
-93
lines changed

2 files changed

+71
-93
lines changed

‎llvm/include/llvm/Analysis/GenericDomTreeUpdater.h‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -151,13 +151,6 @@ class GenericDomTreeUpdater {
151151

152152
/// Apply updates that the critical edge (FromBB, ToBB) has been
153153
/// split with NewBB.
154-
///
155-
/// \note Do not use this method with regular edges.
156-
///
157-
/// \note This kind updates are incompatible with generic updates,
158-
/// call this method will submit all generic updates in lazy mode.
159-
/// It is not recommended to interleave applyUpdates and
160-
/// applyUpdatesForCriticalEdgeSplitting.
161154
void splitCriticalEdge(BasicBlockT *FromBB, BasicBlockT *ToBB,
162155
BasicBlockT *NewBB);
163156

‎llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h‎

Lines changed: 71 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -276,14 +276,14 @@ void GenericDomTreeUpdater<DerivedT, DomTreeT,
276276
assert(I < E && "Iterator range invalid; there should be DomTree updates.");
277277
if (!I->IsCriticalEdgeSplit) {
278278
SmallVector<UpdateT, 32> NormalUpdates;
279-
while (I != E && !I->IsCriticalEdgeSplit)
280-
NormalUpdates.push_back((I++)->Update);
279+
for (; I != E && !I->IsCriticalEdgeSplit; ++I)
280+
NormalUpdates.push_back(I->Update);
281281
DT->applyUpdates(NormalUpdates);
282282
PendDTUpdateIndex += NormalUpdates.size();
283283
} else {
284-
SmallVector<CriticalEdge, 32> CriticalEdges;
285-
while (I != E && I->IsCriticalEdgeSplit)
286-
CriticalEdges.push_back((I++)->EdgeSplit);
284+
SmallVector<CriticalEdge> CriticalEdges;
285+
for (; I != E && I->IsCriticalEdgeSplit; ++I)
286+
CriticalEdges.push_back(I->EdgeSplit);
287287
splitDTCriticalEdges(CriticalEdges);
288288
PendDTUpdateIndex += CriticalEdges.size();
289289
}
@@ -305,14 +305,14 @@ void GenericDomTreeUpdater<DerivedT, DomTreeT,
305305
"Iterator range invalid; there should be PostDomTree updates.");
306306
if (!I->IsCriticalEdgeSplit) {
307307
SmallVector<UpdateT, 32> NormalUpdates;
308-
while (I != E && !I->IsCriticalEdgeSplit)
309-
NormalUpdates.push_back((I++)->Update);
308+
for (; I != E && !I->IsCriticalEdgeSplit; ++I)
309+
NormalUpdates.push_back(I->Update);
310310
PDT->applyUpdates(NormalUpdates);
311311
PendPDTUpdateIndex += NormalUpdates.size();
312312
} else {
313-
SmallVector<CriticalEdge, 32> CriticalEdges;
314-
while (I != E && I->IsCriticalEdgeSplit)
315-
CriticalEdges.push_back((I++)->EdgeSplit);
313+
SmallVector<CriticalEdge> CriticalEdges;
314+
for (; I != E && I->IsCriticalEdgeSplit; ++I)
315+
CriticalEdges.push_back(I->EdgeSplit);
316316
splitPDTCriticalEdges(CriticalEdges);
317317
PendPDTUpdateIndex += CriticalEdges.size();
318318
}
@@ -393,7 +393,7 @@ template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
393393
void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::
394394
splitDTCriticalEdges(ArrayRef<CriticalEdge> Edges) {
395395
// Bail out early if there is nothing to do.
396-
if (Edges.empty())
396+
if (!DT || Edges.empty())
397397
return;
398398

399399
// Remember all the basic blocks that are inserted during
@@ -410,107 +410,92 @@ void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::
410410
// index, i.e., the information for the ith element of Edges is
411411
// the ith element of IsNewIDom.
412412
SmallBitVector IsNewIDom(Edges.size(), true);
413-
size_t Idx = 0;
414413

415414
// Collect all the dominance properties info, before invalidating
416415
// the underlying DT.
417-
for (const auto &Edge : Edges) {
416+
for (const auto &[Idx, Edge] : enumerate(Edges)) {
418417
// Update dominator information.
419-
if (DT) {
420-
BasicBlockT *Succ = Edge.ToBB;
421-
auto *SuccDTNode = DT->getNode(Succ);
422-
423-
for (BasicBlockT *PredBB : predecessors(Succ)) {
424-
if (PredBB == Edge.NewBB)
425-
continue;
426-
// If we are in this situation:
427-
// FromBB1 FromBB2
428-
// + +
429-
// + + + +
430-
// + + + +
431-
// ... Split1 Split2 ...
432-
// + +
433-
// + +
434-
// +
435-
// Succ
436-
// Instead of checking the domiance property with Split2, we check it
437-
// with FromBB2 since Split2 is still unknown of the underlying DT
438-
// structure.
439-
if (NewBBs.count(PredBB)) {
440-
assert(pred_size(PredBB) == 1 && "A basic block resulting from a "
441-
"critical edge split has more "
442-
"than one predecessor!");
443-
PredBB = *pred_begin(PredBB);
444-
}
445-
if (!DT->dominates(SuccDTNode, DT->getNode(PredBB))) {
446-
IsNewIDom[Idx] = false;
447-
break;
448-
}
418+
BasicBlockT *Succ = Edge.ToBB;
419+
auto *SuccDTNode = DT->getNode(Succ);
420+
421+
for (BasicBlockT *PredBB : predecessors(Succ)) {
422+
if (PredBB == Edge.NewBB)
423+
continue;
424+
// If we are in this situation:
425+
// FromBB1 FromBB2
426+
// + +
427+
// + + + +
428+
// + + + +
429+
// ... Split1 Split2 ...
430+
// + +
431+
// + +
432+
// +
433+
// Succ
434+
// Instead of checking the domiance property with Split2, we check it
435+
// with FromBB2 since Split2 is still unknown of the underlying DT
436+
// structure.
437+
if (NewBBs.contains(PredBB)) {
438+
assert(pred_size(PredBB) == 1 && "A basic block resulting from a "
439+
"critical edge split has more "
440+
"than one predecessor!");
441+
PredBB = *pred_begin(PredBB);
442+
}
443+
if (!DT->dominates(SuccDTNode, DT->getNode(PredBB))) {
444+
IsNewIDom[Idx] = false;
445+
break;
449446
}
450447
}
451-
++Idx;
452448
}
453449

454450
// Now, update DT with the collected dominance properties info.
455-
Idx = 0;
456-
for (const auto &Edge : Edges) {
457-
if (DT) {
458-
// We know FromBB dominates NewBB.
459-
auto *NewDTNode = DT->addNewBlock(Edge.NewBB, Edge.FromBB);
460-
461-
// If all the other predecessors of "Succ" are dominated by "Succ" itself
462-
// then the new block is the new immediate dominator of "Succ". Otherwise,
463-
// the new block doesn't dominate anything.
464-
if (IsNewIDom[Idx])
465-
DT->changeImmediateDominator(DT->getNode(Edge.ToBB), NewDTNode);
466-
}
467-
++Idx;
451+
for (const auto &[Idx, Edge] : enumerate(Edges)) {
452+
// We know FromBB dominates NewBB.
453+
auto *NewDTNode = DT->addNewBlock(Edge.NewBB, Edge.FromBB);
454+
455+
// If all the other predecessors of "Succ" are dominated by "Succ" itself
456+
// then the new block is the new immediate dominator of "Succ". Otherwise,
457+
// the new block doesn't dominate anything.
458+
if (IsNewIDom[Idx])
459+
DT->changeImmediateDominator(DT->getNode(Edge.ToBB), NewDTNode);
468460
}
469461
}
470462

471463
template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
472464
void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::
473465
splitPDTCriticalEdges(ArrayRef<CriticalEdge> Edges) {
474466
// Bail out early if there is nothing to do.
475-
if (Edges.empty())
467+
if (!PDT || Edges.empty())
476468
return;
477469

478470
SmallBitVector IsNewIPDom(Edges.size(), true);
479-
SmallSet<BasicBlockT *, 32> NewBBs;
471+
SmallSet<BasicBlockT *, 8> NewBBs;
480472
for (const auto &Edge : Edges)
481473
NewBBs.insert(Edge.NewBB);
482-
size_t Idx = 0;
483-
for (const auto &Edge : Edges) {
474+
475+
for (const auto &[Idx, Edge] : enumerate(Edges)) {
484476
// Same as DT version but from another direction.
485-
if (PDT) {
486-
BasicBlockT *Pred = Edge.FromBB;
487-
auto *PredDTNode = PDT->getNode(Pred);
488-
for (BasicBlockT *SuccBB : successors(Pred)) {
489-
if (SuccBB == Edge.NewBB)
490-
continue;
491-
if (NewBBs.count(SuccBB)) {
492-
assert(succ_size(SuccBB) == 1 && "A basic block resulting from a "
493-
"critical edge split has more "
494-
"than one predecessor!");
495-
SuccBB = *succ_begin(SuccBB);
496-
}
497-
if (!PDT->dominates(PredDTNode, PDT->getNode(SuccBB))) {
498-
IsNewIPDom[Idx] = false;
499-
break;
500-
}
477+
BasicBlockT *Pred = Edge.FromBB;
478+
auto *PredDTNode = PDT->getNode(Pred);
479+
for (BasicBlockT *SuccBB : successors(Pred)) {
480+
if (SuccBB == Edge.NewBB)
481+
continue;
482+
if (NewBBs.count(SuccBB)) {
483+
assert(succ_size(SuccBB) == 1 && "A basic block resulting from a "
484+
"critical edge split has more "
485+
"than one predecessor!");
486+
SuccBB = *succ_begin(SuccBB);
487+
}
488+
if (!PDT->dominates(PredDTNode, PDT->getNode(SuccBB))) {
489+
IsNewIPDom[Idx] = false;
490+
break;
501491
}
502492
}
503-
++Idx;
504493
}
505494

506-
Idx = 0;
507-
for (const auto &Edge : Edges) {
508-
if (PDT) {
509-
auto *NewPDTNode = PDT->addNewBlock(Edge.NewBB, Edge.ToBB);
510-
if (IsNewIPDom[Idx])
511-
PDT->changeImmediateDominator(PDT->getNode(Edge.FromBB), NewPDTNode);
512-
}
513-
++Idx;
495+
for (const auto &[Idx, Edge] : enumerate(Edges)) {
496+
auto *NewPDTNode = PDT->addNewBlock(Edge.NewBB, Edge.ToBB);
497+
if (IsNewIPDom[Idx])
498+
PDT->changeImmediateDominator(PDT->getNode(Edge.FromBB), NewPDTNode);
514499
}
515500
}
516501

0 commit comments

Comments
 (0)