[llvm] r323034 - [Dominators] Fix some edge cases for PostDomTree updating

[Hans Wennborg via llvm-commits]

Merged to 6.0.0 in r323121.

(That also accidentally included r322905, which was supposed to get
merged separately. We can tease them apart if we need to roll anything
back.)

On Sat, Jan 20, 2018 at 11:29 AM, David Green via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: dmgreen
> Date: Sat Jan 20 02:29:37 2018
> New Revision: 323034
>
> URL: http://llvm.org/viewvc/llvm-project?rev=323034&view=rev
> Log:
> [Dominators] Fix some edge cases for PostDomTree updating
>
> These fix some odd cfg cases where batch-updating the post
> dom tree fails. Usually around infinite loops and roots
> ending up being different.
>
> Differential Revision: https://reviews.llvm.org/D42247
>
>
> Modified:
>     llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
>     llvm/trunk/unittests/IR/DominatorTreeBatchUpdatesTest.cpp
>
> Modified: llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h?rev=323034&r1=323033&r2=323034&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h (original)
> +++ llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h Sat Jan 20 02:29:37 2018
> @@ -706,7 +706,7 @@ struct SemiNCAInfo {
>        // algorithm does not really know or use the set of roots and can make a
>        // different (implicit) decision about which nodes within an infinite loop
>        // becomes a root.
> -      if (DT.isVirtualRoot(TN->getIDom())) {
> +      if (TN && !DT.isVirtualRoot(TN->getIDom())) {
>          DEBUG(dbgs() << "Root " << BlockNamePrinter(R)
>                       << " is not virtual root's child\n"
>                       << "The entire tree needs to be rebuilt\n");
> @@ -940,21 +940,21 @@ struct SemiNCAInfo {
>      const NodePtr NCDBlock = DT.findNearestCommonDominator(From, To);
>      const TreeNodePtr NCD = DT.getNode(NCDBlock);
>
> -    // To dominates From -- nothing to do.
> -    if (ToTN == NCD) return;
> -
> -    DT.DFSInfoValid = false;
> -
> -    const TreeNodePtr ToIDom = ToTN->getIDom();
> -    DEBUG(dbgs() << "\tNCD " << BlockNamePrinter(NCD) << ", ToIDom "
> -                 << BlockNamePrinter(ToIDom) << "\n");
> -
> -    // To remains reachable after deletion.
> -    // (Based on the caption under Figure 4. from the second paper.)
> -    if (FromTN != ToIDom || HasProperSupport(DT, BUI, ToTN))
> -      DeleteReachable(DT, BUI, FromTN, ToTN);
> -    else
> -      DeleteUnreachable(DT, BUI, ToTN);
> +    // If To dominates From -- nothing to do.
> +    if (ToTN != NCD) {
> +      DT.DFSInfoValid = false;
> +
> +      const TreeNodePtr ToIDom = ToTN->getIDom();
> +      DEBUG(dbgs() << "\tNCD " << BlockNamePrinter(NCD) << ", ToIDom "
> +                   << BlockNamePrinter(ToIDom) << "\n");
> +
> +      // To remains reachable after deletion.
> +      // (Based on the caption under Figure 4. from the second paper.)
> +      if (FromTN != ToIDom || HasProperSupport(DT, BUI, ToTN))
> +        DeleteReachable(DT, BUI, FromTN, ToTN);
> +      else
> +        DeleteUnreachable(DT, BUI, ToTN);
> +    }
>
>      if (IsPostDom) UpdateRootsAfterUpdate(DT, BUI);
>    }
>
> Modified: llvm/trunk/unittests/IR/DominatorTreeBatchUpdatesTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/DominatorTreeBatchUpdatesTest.cpp?rev=323034&r1=323033&r2=323034&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/IR/DominatorTreeBatchUpdatesTest.cpp (original)
> +++ llvm/trunk/unittests/IR/DominatorTreeBatchUpdatesTest.cpp Sat Jan 20 02:29:37 2018
> @@ -258,3 +258,98 @@ TEST(DominatorTreeBatchUpdates, InsertDe
>      EXPECT_TRUE(PDT.verify());
>    }
>  }
> +
> +// These are some odd flowgraphs, usually generated from csmith cases,
> +// which are difficult on post dom trees.
> +TEST(DominatorTreeBatchUpdates, InfiniteLoop) {
> +  std::vector<CFGBuilder::Arc> Arcs = {
> +      {"1", "2"},
> +      {"2", "3"},
> +      {"3", "6"}, {"3", "5"},
> +      {"4", "5"},
> +      {"5", "2"},
> +      {"6", "3"}, {"6", "4"}};
> +
> +  // SplitBlock on 3 -> 5
> +  std::vector<CFGBuilder::Update> Updates = {
> +      {CFGInsert, {"N", "5"}},  {CFGInsert, {"3", "N"}}, {CFGDelete, {"3", "5"}}};
> +
> +  CFGHolder Holder;
> +  CFGBuilder B(Holder.F, Arcs, Updates);
> +  DominatorTree DT(*Holder.F);
> +  EXPECT_TRUE(DT.verify());
> +  PostDomTree PDT(*Holder.F);
> +  EXPECT_TRUE(PDT.verify());
> +
> +  while (B.applyUpdate())
> +    ;
> +
> +  auto DomUpdates = ToDomUpdates(B, Updates);
> +  DT.applyUpdates(DomUpdates);
> +  EXPECT_TRUE(DT.verify());
> +  PDT.applyUpdates(DomUpdates);
> +  EXPECT_TRUE(PDT.verify());
> +}
> +
> +TEST(DominatorTreeBatchUpdates, DeadBlocks) {
> +  std::vector<CFGBuilder::Arc> Arcs = {
> +      {"1", "2"},
> +      {"2", "3"},
> +      {"3", "4"}, {"3", "7"},
> +      {"4", "4"},
> +      {"5", "6"}, {"5", "7"},
> +      {"6", "7"},
> +      {"7", "2"}, {"7", "8"}};
> +
> +  // Remove dead 5 and 7,
> +  // plus SplitBlock on 7 -> 8
> +  std::vector<CFGBuilder::Update> Updates = {
> +      {CFGDelete, {"6", "7"}},  {CFGDelete, {"5", "7"}}, {CFGDelete, {"5", "6"}},
> +      {CFGInsert, {"N", "8"}},  {CFGInsert, {"7", "N"}}, {CFGDelete, {"7", "8"}}};
> +
> +  CFGHolder Holder;
> +  CFGBuilder B(Holder.F, Arcs, Updates);
> +  DominatorTree DT(*Holder.F);
> +  EXPECT_TRUE(DT.verify());
> +  PostDomTree PDT(*Holder.F);
> +  EXPECT_TRUE(PDT.verify());
> +
> +  while (B.applyUpdate())
> +    ;
> +
> +  auto DomUpdates = ToDomUpdates(B, Updates);
> +  DT.applyUpdates(DomUpdates);
> +  EXPECT_TRUE(DT.verify());
> +  PDT.applyUpdates(DomUpdates);
> +  EXPECT_TRUE(PDT.verify());
> +}
> +
> +TEST(DominatorTreeBatchUpdates, InfiniteLoop2) {
> +  std::vector<CFGBuilder::Arc> Arcs = {
> +      {"1", "2"},
> +      {"2", "6"}, {"2", "3"},
> +      {"3", "4"},
> +      {"4", "5"}, {"4", "6"},
> +      {"5", "4"},
> +      {"6", "2"}};
> +
> +  // SplitBlock on 4 -> 6
> +  std::vector<CFGBuilder::Update> Updates = {
> +      {CFGInsert, {"N", "6"}},  {CFGInsert, {"4", "N"}}, {CFGDelete, {"4", "6"}}};
> +
> +  CFGHolder Holder;
> +  CFGBuilder B(Holder.F, Arcs, Updates);
> +  DominatorTree DT(*Holder.F);
> +  EXPECT_TRUE(DT.verify());
> +  PostDomTree PDT(*Holder.F);
> +  EXPECT_TRUE(PDT.verify());
> +
> +  while (B.applyUpdate())
> +    ;
> +
> +  auto DomUpdates = ToDomUpdates(B, Updates);
> +  DT.applyUpdates(DomUpdates);
> +  EXPECT_TRUE(DT.verify());
> +  PDT.applyUpdates(DomUpdates);
> +  EXPECT_TRUE(PDT.verify());
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits