[llvm] r316582 - [Local] Fix a bug in the domtree update logic for MergeBasicBlockIntoOnlyPred.
Matthew Simpson via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 25 15:53:49 PDT 2017
This failure may have been my fault - I don't think Balaram's patch should
have caused it. It's possible the test may be failing sporadically. I've
attempted to fix it in r316625.
-- Matt
On Wed, Oct 25, 2017 at 5:38 PM, Balaram Makam via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> I wasn’t able to reproduce the failure, so I reverted the commit in
> r316612 while I investigate.
>
> Can you please send me the command line to use that will fail under
> bugpoint?
>
>
>
> Thanks,
>
> Balaram
>
>
>
> *From:* Balaram Makam [mailto:bmakam at codeaurora.org]
> *Sent:* Wednesday, October 25, 2017 4:04 PM
> *To:* 'Galina Kistanova' <gkistanova at gmail.com>
> *Cc:* 'Artur Pilipenko via llvm-commits' <llvm-commits at lists.llvm.org>
> *Subject:* RE: [llvm] r316582 - [Local] Fix a bug in the domtree update
> logic for MergeBasicBlockIntoOnlyPred.
>
>
>
> Looking..
>
>
>
> *From:* Galina Kistanova [mailto:gkistanova at gmail.com
> <gkistanova at gmail.com>]
> *Sent:* Wednesday, October 25, 2017 3:48 PM
> *To:* Balaram Makam <bmakam at codeaurora.org>
> *Cc:* Artur Pilipenko via llvm-commits <llvm-commits at lists.llvm.org>
> *Subject:* Re: [llvm] r316582 - [Local] Fix a bug in the domtree update
> logic for MergeBasicBlockIntoOnlyPred.
>
>
>
> 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
>
>
>
> _______________________________________________
> 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/b0f32b9f/attachment.html>
More information about the llvm-commits
mailing list