[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 17:10:20 PDT 2017
I believe my patch should have fixed the stability issue. Balaram, would
you mind reapplying your patch? Please let me know if this test fails again.
-- Matt
On Wed, Oct 25, 2017 at 6:53 PM, Matthew Simpson <mssimpso at codeaurora.org>
wrote:
> 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-expensiv
>> e-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/Transform
>> s/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/Tra
>> nsforms/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>(SinglePre
>> d->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/c9d14730/attachment.html>
More information about the llvm-commits
mailing list