[llvm] r316582 - [Local] Fix a bug in the domtree update logic for MergeBasicBlockIntoOnlyPred.

[Galina Kistanova via llvm-commits]

Hello Balaram,

It looks like this commit broke tests on our builder:
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/5719

. . .
Failing Tests (1):
    LLVM :: Transforms/CalledValuePropagation/simple-arguments.ll

Please have a look?

Thanks

Galina

On Wed, Oct 25, 2017 at 7:55 AM, Balaram Makam via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: bmakam
> Date: Wed Oct 25 07:55:48 2017
> New Revision: 316582
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316582&view=rev
> Log:
> [Local] Fix a bug in the domtree update logic for
> MergeBasicBlockIntoOnlyPred.
>
> Summary: For some irreducible CFG the domtree nodes might be dead, do not
> update domtree for dead nodes.
>
> Reviewers: kuhar, dberlin, hfinkel
>
> Reviewed By: kuhar
>
> Subscribers: llvm-commits, mcrosier
>
> Differential Revision: https://reviews.llvm.org/D38960
>
> Modified:
>     llvm/trunk/lib/Transforms/Utils/Local.cpp
>     llvm/trunk/unittests/Transforms/Utils/Local.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/Local.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> Transforms/Utils/Local.cpp?rev=316582&r1=316581&r2=316582&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/Local.cpp Wed Oct 25 07:55:48 2017
> @@ -649,9 +649,13 @@ void llvm::MergeBasicBlockIntoOnlyPred(B
>      DestBB->moveAfter(PredBB);
>
>    if (DT) {
> -    BasicBlock *PredBBIDom = DT->getNode(PredBB)->getIDom()->getBlock();
> -    DT->changeImmediateDominator(DestBB, PredBBIDom);
> -    DT->eraseNode(PredBB);
> +    // For some irreducible CFG we end up having forward-unreachable
> blocks
> +    // so check if getNode returns a valid node before updating the
> domtree.
> +    if (DomTreeNode *DTN = DT->getNode(PredBB)) {
> +      BasicBlock *PredBBIDom = DTN->getIDom()->getBlock();
> +      DT->changeImmediateDominator(DestBB, PredBBIDom);
> +      DT->eraseNode(PredBB);
> +    }
>    }
>    // Nuke BB.
>    PredBB->eraseFromParent();
>
> Modified: llvm/trunk/unittests/Transforms/Utils/Local.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/
> Transforms/Utils/Local.cpp?rev=316582&r1=316581&r2=316582&view=diff
> ============================================================
> ==================
> --- llvm/trunk/unittests/Transforms/Utils/Local.cpp (original)
> +++ llvm/trunk/unittests/Transforms/Utils/Local.cpp Wed Oct 25 07:55:48
> 2017
> @@ -166,3 +166,48 @@ TEST(Local, ReplaceDbgDeclare) {
>        Declares++;
>    EXPECT_EQ(2, Declares);
>  }
> +
> +/// Build the dominator tree for the function and run the Test.
> +static void runWithDomTree(
> +    Module &M, StringRef FuncName,
> +    function_ref<void(Function &F, DominatorTree *DT)> Test) {
> +  auto *F = M.getFunction(FuncName);
> +  ASSERT_NE(F, nullptr) << "Could not find " << FuncName;
> +  // Compute the dominator tree for the function.
> +  DominatorTree DT(*F);
> +  Test(*F, &DT);
> +}
> +
> +TEST(Local, MergeBasicBlockIntoOnlyPred) {
> +  LLVMContext C;
> +
> +  std::unique_ptr<Module> M = parseIR(
> +      C,
> +      "define i32 @f(i8* %str) {\n"
> +      "entry:\n"
> +      "  br label %bb2.i\n"
> +      "bb2.i:                                            ; preds =
> %bb4.i, %entry\n"
> +      "  br i1 false, label %bb4.i, label %base2flt.exit204\n"
> +      "bb4.i:                                            ; preds =
> %bb2.i\n"
> +      "  br i1 false, label %base2flt.exit204, label %bb2.i\n"
> +      "bb10.i196.bb7.i197_crit_edge:                     ; No
> predecessors!\n"
> +      "  br label %bb7.i197\n"
> +      "bb7.i197:                                         ; preds =
> %bb10.i196.bb7.i197_crit_edge\n"
> +      "  %.reg2mem.0 = phi i32 [ %.reg2mem.0,
> %bb10.i196.bb7.i197_crit_edge ]\n"
> +      "  br i1 undef, label %base2flt.exit204, label %base2flt.exit204\n"
> +      "base2flt.exit204:                                 ; preds =
> %bb7.i197, %bb7.i197, %bb2.i, %bb4.i\n"
> +      "  ret i32 0\n"
> +      "}\n");
> +  runWithDomTree(
> +      *M, "f", [&](Function &F, DominatorTree *DT) {
> +        for (Function::iterator I = F.begin(), E = F.end(); I != E;) {
> +          BasicBlock *BB = &*I++;
> +          BasicBlock *SinglePred = BB->getSinglePredecessor();
> +          if (!SinglePred || SinglePred == BB || BB->hasAddressTaken())
> continue;
> +          BranchInst *Term = dyn_cast<BranchInst>(
> SinglePred->getTerminator());
> +          if (Term && !Term->isConditional())
> +            MergeBasicBlockIntoOnlyPred(BB, DT);
> +        }
> +        EXPECT_TRUE(DT->verify());
> +      });
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171025/4635a631/attachment.html>