<div dir="ltr">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.<div><br></div><div>-- Matt</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 25, 2017 at 6:53 PM, Matthew Simpson <span dir="ltr"><<a href="mailto:mssimpso@codeaurora.org" target="_blank">mssimpso@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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 r<span style="font-size:12.8px">316625.</span><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">-- Matt</span></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 25, 2017 at 5:38 PM, Balaram Makam via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div class="m_-9009511287020159058m_-1573406461366937868WordSection1"><p class="MsoNormal">I wasn’t able to reproduce the failure, so I reverted the commit in r316612 while I investigate.<u></u><u></u></p><p class="MsoNormal">Can you please send me the command line to use that will fail under bugpoint?<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">Thanks,<u></u><u></u></p><p class="MsoNormal">Balaram<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><div><div style="border:none;border-top:solid #e1e1e1 1.0pt;padding:3.0pt 0in 0in 0in"><p class="MsoNormal"><b>From:</b> Balaram Makam [mailto:<a href="mailto:bmakam@codeaurora.org" target="_blank">bmakam@codeaurora.org</a>] <br><b>Sent:</b> Wednesday, October 25, 2017 4:04 PM<br><b>To:</b> 'Galina Kistanova' <<a href="mailto:gkistanova@gmail.com" target="_blank">gkistanova@gmail.com</a>><span><br><b>Cc:</b> 'Artur Pilipenko via llvm-commits' <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>><br></span><b>Subject:</b> RE: [llvm] r316582 - [Local] Fix a bug in the domtree update logic for MergeBasicBlockIntoOnlyPred.<u></u><u></u></p></div></div><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">Looking..<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal"><span><b>From:</b> Galina Kistanova [<a href="mailto:gkistanova@gmail.com" target="_blank">mailto:gkistanova@gmail.com</a>] <br><b>Sent:</b> Wednesday, October 25, 2017 3:48 PM<br><b>To:</b> Balaram Makam <<a href="mailto:bmakam@codeaurora.org" target="_blank">bmakam@codeaurora.org</a>><br><b>Cc:</b> Artur Pilipenko via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>><br></span><b>Subject:</b> Re: [llvm] r316582 - [Local] Fix a bug in the domtree update logic for MergeBasicBlockIntoOnlyPred.<u></u><u></u></p><div><div class="m_-9009511287020159058h5"><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal">Hello Balaram,<br><br>It looks like this commit broke tests on our builder:<br><a href="http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/5719" target="_blank">http://lab.llvm.org:8011/build<wbr>ers/llvm-clang-x86_64-expensiv<wbr>e-checks-win/builds/5719</a><br><br>. . .<br>Failing Tests (1):<br> LLVM :: Transforms/CalledValuePropagat<wbr>ion/simple-arguments.ll<br><br>Please have a look?<br><br>Thanks<br><br>Galina<u></u><u></u></p></div><div><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal">On Wed, Oct 25, 2017 at 7:55 AM, Balaram Makam via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<u></u><u></u></p><blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt"><p class="MsoNormal">Author: bmakam<br>Date: Wed Oct 25 07:55:48 2017<br>New Revision: 316582<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=316582&view=rev" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=316582&view=rev</a><br>Log:<br>[Local] Fix a bug in the domtree update logic for MergeBasicBlockIntoOnlyPred.<br><br>Summary: For some irreducible CFG the domtree nodes might be dead, do not update domtree for dead nodes.<br><br>Reviewers: kuhar, dberlin, hfinkel<br><br>Reviewed By: kuhar<br><br>Subscribers: llvm-commits, mcrosier<br><br>Differential Revision: <a href="https://reviews.llvm.org/D38960" target="_blank">https://reviews.llvm.org/D3896<wbr>0</a><br><br>Modified:<br> llvm/trunk/lib/Transforms/Util<wbr>s/Local.cpp<br> llvm/trunk/unittests/Transform<wbr>s/Utils/Local.cpp<br><br>Modified: llvm/trunk/lib/Transforms/Util<wbr>s/Local.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Local.cpp?rev=316582&r1=316581&r2=316582&view=diff" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Transform<wbr>s/Utils/Local.cpp?rev=316582&<wbr>r1=316581&r2=316582&view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- llvm/trunk/lib/Transforms/Util<wbr>s/Local.cpp (original)<br>+++ llvm/trunk/lib/Transforms/Util<wbr>s/Local.cpp Wed Oct 25 07:55:48 2017<br>@@ -649,9 +649,13 @@ void llvm::MergeBasicBlockIntoOnlyP<wbr>red(B<br> DestBB->moveAfter(PredBB);<br><br> if (DT) {<br>- BasicBlock *PredBBIDom = DT->getNode(PredBB)->getIDom()<wbr>->getBlock();<br>- DT->changeImmediateDominator(D<wbr>estBB, PredBBIDom);<br>- DT->eraseNode(PredBB);<br>+ // For some irreducible CFG we end up having forward-unreachable blocks<br>+ // so check if getNode returns a valid node before updating the domtree.<br>+ if (DomTreeNode *DTN = DT->getNode(PredBB)) {<br>+ BasicBlock *PredBBIDom = DTN->getIDom()->getBlock();<br>+ DT->changeImmediateDominator(D<wbr>estBB, PredBBIDom);<br>+ DT->eraseNode(PredBB);<br>+ }<br> }<br> // Nuke BB.<br> PredBB->eraseFromParent();<br><br>Modified: llvm/trunk/unittests/Transform<wbr>s/Utils/Local.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/Local.cpp?rev=316582&r1=316581&r2=316582&view=diff" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/unittests/Tra<wbr>nsforms/Utils/Local.cpp?rev=<wbr>316582&r1=316581&r2=316582&<wbr>view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- llvm/trunk/unittests/Transform<wbr>s/Utils/Local.cpp (original)<br>+++ llvm/trunk/unittests/Transform<wbr>s/Utils/Local.cpp Wed Oct 25 07:55:48 2017<br>@@ -166,3 +166,48 @@ TEST(Local, ReplaceDbgDeclare) {<br> Declares++;<br> EXPECT_EQ(2, Declares);<br> }<br>+<br>+/// Build the dominator tree for the function and run the Test.<br>+static void runWithDomTree(<br>+ Module &M, StringRef FuncName,<br>+ function_ref<void(Function &F, DominatorTree *DT)> Test) {<br>+ auto *F = M.getFunction(FuncName);<br>+ ASSERT_NE(F, nullptr) << "Could not find " << FuncName;<br>+ // Compute the dominator tree for the function.<br>+ DominatorTree DT(*F);<br>+ Test(*F, &DT);<br>+}<br>+<br>+TEST(Local, MergeBasicBlockIntoOnlyPred) {<br>+ LLVMContext C;<br>+<br>+ std::unique_ptr<Module> M = parseIR(<br>+ C,<br>+ "define i32 @f(i8* %str) {\n"<br>+ "entry:\n"<br>+ " br label %bb2.i\n"<br>+ "bb2.i: ; preds = %bb4.i, %entry\n"<br>+ " br i1 false, label %bb4.i, label %base2flt.exit204\n"<br>+ "bb4.i: ; preds = %bb2.i\n"<br>+ " br i1 false, label %base2flt.exit204, label %bb2.i\n"<br>+ "bb10.i196.bb7.i197_crit_edge:<wbr> ; No predecessors!\n"<br>+ " br label %bb7.i197\n"<br>+ "bb7.i197: ; preds = %bb10.i196.bb7.i197_crit_edge\<wbr>n"<br>+ " %.reg2mem.0 = phi i32 [ %.reg2mem.0, %bb10.i196.bb7.i197_crit_edge ]\n"<br>+ " br i1 undef, label %base2flt.exit204, label %base2flt.exit204\n"<br>+ "base2flt.exit204: ; preds = %bb7.i197, %bb7.i197, %bb2.i, %bb4.i\n"<br>+ " ret i32 0\n"<br>+ "}\n");<br>+ runWithDomTree(<br>+ *M, "f", [&](Function &F, DominatorTree *DT) {<br>+ for (Function::iterator I = F.begin(), E = F.end(); I != E;) {<br>+ BasicBlock *BB = &*I++;<br>+ BasicBlock *SinglePred = BB->getSinglePredecessor();<br>+ if (!SinglePred || SinglePred == BB || BB->hasAddressTaken()) continue;<br>+ BranchInst *Term = dyn_cast<BranchInst>(SinglePre<wbr>d->getTerminator());<br>+ if (Term && !Term->isConditional())<br>+ MergeBasicBlockIntoOnlyPred(BB<wbr>, DT);<br>+ }<br>+ EXPECT_TRUE(DT->verify());<br>+ });<br>+}<br><br><br>______________________________<wbr>_________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><u></u><u></u></p></blockquote></div><p class="MsoNormal"><u></u> <u></u></p></div></div></div></div></div><br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div>