llvm.org GIT mirror llvm / 31b1598
[Dominators] Always recalculate postdominators when update yields different roots Summary: This patch makes postdominators always recalculate the tree when an update causes to change the tree roots. As @dmgreen noticed in [[ https://reviews.llvm.org/D41298 | D41298 ]], the previous implementation was not conservative enough and it was possible to end up with a PostDomTree that was different than a freshly computed one. The patch also compares postdominators with a freshly computed tree at the end of full verification to make sure we don't hit similar issues in the future. This should (ideally) be also backported to 6.0 before the release, although I don't have any reports of this causing an observable error. It should be safe to do it even if it's late in the release, as the change only makes the current behavior more conservative. Reviewers: dmgreen, dberlin, davide, brzycki, grosser Reviewed By: brzycki, grosser Subscribers: llvm-commits, dmgreen Differential Revision: https://reviews.llvm.org/D43140 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@324962 91177308-0d34-0410-b5e6-96231b3b80d8 Jakub Kuderski 2 years ago
1 changed file(s) with 25 addition(s) and 20 deletion(s). Raw diff Collapse all Expand all
697697 return;
698698
699699 // Recalculate the set of roots.
700 DT.Roots = FindRoots(DT, BUI);
701 for (const NodePtr R : DT.Roots) {
702 const TreeNodePtr TN = DT.getNode(R);
703 // A CFG node was selected as a tree root, but the corresponding tree node
704 // is not connected to the virtual root. This is because the incremental
705 // algorithm does not really know or use the set of roots and can make a
706 // different (implicit) decision about which nodes within an infinite loop
707 // becomes a root.
708 if (TN && !DT.isVirtualRoot(TN->getIDom())) {
709 DEBUG(dbgs() << "Root " << BlockNamePrinter(R)
710 << " is not virtual root's child\n"
711 << "The entire tree needs to be rebuilt\n");
712 // It should be possible to rotate the subtree instead of recalculating
713 // the whole tree, but this situation happens extremely rarely in
714 // practice.
715 CalculateFromScratch(DT, BUI);
716 return;
717 }
700 auto Roots = FindRoots(DT, BUI);
701 if (DT.Roots.size() != Roots.size() ||
702 !std::is_permutation(DT.Roots.begin(), DT.Roots.end(), Roots.begin())) {
703 // The roots chosen in the CFG have changed. This is because the
704 // incremental algorithm does not really know or use the set of roots and
705 // can make a different (implicit) decision about which node within an
706 // infinite loop becomes a root.
707
708 DEBUG(dbgs() << "Roots are different in updated trees\n"
709 << "The entire tree needs to be rebuilt\n");
710 // It may be possible to update the tree without recalculating it, but
711 // we do not know yet how to do it, and it happens rarely in practise.
712 CalculateFromScratch(DT, BUI);
713 return;
718714 }
719715 }
720716
16591655 case DomTreeT::VerificationLevel::Basic:
16601656 return SNCA.verifyParentProperty(DT) && SNCA.IsSameAsFreshTree(DT);
16611657
1662 case DomTreeT::VerificationLevel::Full:
1663 return SNCA.verifyParentProperty(DT) && SNCA.verifySiblingProperty(DT);
1658 case DomTreeT::VerificationLevel::Full: {
1659 bool FullRes
1660 = SNCA.verifyParentProperty(DT) && SNCA.verifySiblingProperty(DT);
1661
1662 // Postdominators depend on root selection, make sure that a fresh tree
1663 // looks the same.
1664 if (DT.isPostDominator())
1665 FullRes &= SNCA.IsSameAsFreshTree(DT);
1666
1667 return FullRes;
1668 }
16641669 }
16651670
16661671 llvm_unreachable("Unhandled DomTree VerificationLevel");