<html><body bgcolor="#FFFFFF"><p><font size="2">Got it. Thank you </font>Fedor<font size="2">.</font><br><font size="2"><br>Best,<br><br>Jinsong Ji (纪金松), PhD.<br><br>XL/LLVM on Power Compiler Development <br>E-mail: jji@us.ibm.com</font><br><br><img width="16" height="16" src="cid:1__=8FBB0EF7DFDEBD9D8f9e8a93df938690918c8FB@" border="0" alt="Inactive hide details for Fedor Sergeev ---08/27/2019 06:35:35 PM---Please, find reproducer in the comment to the original revi"><font size="2" color="#424282">Fedor Sergeev ---08/27/2019 06:35:35 PM---Please, find reproducer in the comment to the original review: <a href="https://urldefense.proofpoint.com/">https://urldefense.proofpoint.com/</a></font><br><br><font size="2" color="#5F5F5F">From: </font><font size="2">Fedor Sergeev <fedor.sergeev@azul.com></font><br><font size="2" color="#5F5F5F">To: </font><font size="2">Jinsong Ji <jji@us.ibm.com>, "Mikael Holmén" <mikael.holmen@ericsson.com></font><br><font size="2" color="#5F5F5F">Cc: </font><font size="2">"llvm-commits@lists.llvm.org" <llvm-commits@lists.llvm.org>, llvm-commits <llvm-commits-bounces@lists.llvm.org>, "shkzhang@cn.ibm.com" <shkzhang@cn.ibm.com></font><br><font size="2" color="#5F5F5F">Date: </font><font size="2">08/27/2019 06:35 PM</font><br><font size="2" color="#5F5F5F">Subject: </font><font size="2">[EXTERNAL] Re: [llvm] r369191 - [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks</font><br><hr width="100%" size="2" align="left" noshade style="color:#8091A5; "><br><br><br>Please, find reproducer in the comment to the original review:<br> <a href="https://reviews.llvm.org/D63972#1647948"><u><font color="#0000FF">https://reviews.llvm.org/D63972#1647948</font></u></a><br><br>regards,<br> Fedor.<br><br>On 8/27/19 6:08 PM, Jinsong Ji wrote:
<ul><ul><tt><font size="2">@skatkov is also reporting crash in D63972#1646303<br>Contacted @ZhangKang, and revert the commit on behalf of him in </font></tt><a href="https://reviews.llvm.org/rL370069"><tt><u><font size="2" color="#0000FF">https://reviews.llvm.org/rL370069</font></u></tt></a><tt><font size="2">.</font></tt><br><font size="2"><br>Reproducer is appreciated. Thanks.</font><br><br><font size="2"><br>Best,<br><br>Jinsong Ji (纪金松), PhD.<br><br>XL/LLVM on Power Compiler Development <br>E-mail: </font><a href="mailto:jji@us.ibm.com"><u><font size="2" color="#0000FF">jji@us.ibm.com</font></u></a><br><br><img src="cid:1__=8FBB0EF7DFDEBD9D8f9e8a93df938690918c8FB@" width="16" height="16" alt="Inactive
hide details for "Mikael Holmén via llvm-commits"
---08/27/2019 01:20:47 AM---On Tue, 2019-08-27 at 01:01 +0300,
Fedor"><font size="2" color="#424282">"Mikael Holmén via llvm-commits" ---08/27/2019 01:20:47 AM---On Tue, 2019-08-27 at 01:01 +0300, Fedor Sergeev wrote: > Hmm... we are seeing problems with X86 bac</font><br><font size="2" color="#5F5F5F"><br>From: </font><font size="2">"Mikael Holmén via llvm-commits" </font><a href="mailto:llvm-commits@lists.llvm.org"><u><font size="2" color="#0000FF"><llvm-commits@lists.llvm.org></font></u></a><font size="2" color="#5F5F5F"><br>To: </font><a href="mailto:fedor.sergeev@azul.com"><u><font size="2" color="#0000FF">"fedor.sergeev@azul.com"</font></u></a><font size="2"> </font><a href="mailto:fedor.sergeev@azul.com"><u><font size="2" color="#0000FF"><fedor.sergeev@azul.com></font></u></a><font size="2">, </font><a href="mailto:shkzhang@cn.ibm.com"><u><font size="2" color="#0000FF">"shkzhang@cn.ibm.com"</font></u></a><font size="2"> </font><a href="mailto:shkzhang@cn.ibm.com"><u><font size="2" color="#0000FF"><shkzhang@cn.ibm.com></font></u></a><font size="2" color="#5F5F5F"><br>Cc: </font><a href="mailto:llvm-commits@lists.llvm.org"><u><font size="2" color="#0000FF">"llvm-commits@lists.llvm.org"</font></u></a><font size="2"> </font><a href="mailto:llvm-commits@lists.llvm.org"><u><font size="2" color="#0000FF"><llvm-commits@lists.llvm.org></font></u></a><font size="2" color="#5F5F5F"><br>Date: </font><font size="2">08/27/2019 01:20 AM</font><font size="2" color="#5F5F5F"><br>Subject: </font><font size="2">[EXTERNAL] Re: [llvm] r369191 - [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks</font><font size="2" color="#5F5F5F"><br>Sent by: </font><font size="2">"llvm-commits" </font><a href="mailto:llvm-commits-bounces@lists.llvm.org"><u><font size="2" color="#0000FF"><llvm-commits-bounces@lists.llvm.org></font></u></a><p><hr width="100%" size="2" align="left" noshade><br><br><tt><font size="2"><br>On Tue, 2019-08-27 at 01:01 +0300, Fedor Sergeev wrote:<br>> Hmm... we are seeing problems with X86 backend (and our Java JIT <br>> frontend) that we managed<br>> to bisect down to this change.<br>> I might be able to provide a clean reproducer tomorrow.<br>> <br>> Meanwhile, Mikael, was there a bug filed?<br>> I dont see any followup commits related to this....<br>> <br><br>Hi,<br><br>Turned out my problem was specific to my out-of-tree backend where we<br>run MBP both before and after regalloc.<br><br>When we ran it before RA (which isn't done in-tree anywhere as far as I<br>know) it didn't update PHIs when it changed the CFG and then the<br>verifier complained.<br><br>/Mikael<br><br>> regards,<br>> Fedor.<br>> <br>> <br>> On 8/23/19 4:36 PM, Mikael Holmén via llvm-commits wrote:<br>> > Hi,<br>> > <br>> > We're seeing a problem with this commit for our out-of-tree<br>> > backend.<br>> > <br>> > Before 'Early Branch Probability Basic Block Placement':<br>> > <br>> > bb.2.cond.end.thread:<br>> > ; predecessors: %bb.1<br>> > successors: %bb.5(0x80000000); %bb.5(100.00%)<br>> > <br>> > brr_uncond %bb.5<br>> > <br>> > [...]<br>> > <br>> > bb.5.land.lhs.true28:<br>> > ; predecessors: %bb.2, %bb.3<br>> > successors: %bb.6(0x30000000), %bb.7(0x50000000);<br>> > %bb.6(37.50%),<br>> > %bb.7(62.50%)<br>> > <br>> > %8:an32_rn_pairs = ...<br>> > cmp ...<br>> > brr_cond %bb.7<br>> > brr_uncond %bb.6<br>> > <br>> > [...]<br>> > <br>> > bb.7.cond.end38:<br>> > ; predecessors: %bb.5, %bb.4, %bb.6<br>> > successors: %bb.8(0x00000001), %bb.9(0x7fffffff); %bb.8(0.00%),<br>> > %bb.9(100.00%)<br>> > <br>> > %2:an32_0_7 = PHI %8:an32_rn_pairs, %bb.5, %11:an32_rn_pairs,<br>> > %bb.4,<br>> > %0:an32_rn_pairs, %bb.6<br>> > cmps ...<br>> > brr_cond %bb.9<br>> > brr_uncond %bb.8<br>> > <br>> > <br>> > and after:<br>> > <br>> > bb.2.cond.end.thread:<br>> > ; predecessors: %bb.1, %bb.3<br>> > successors: %bb.7(0x50000000), %bb.6(0x30000000);<br>> > %bb.7(62.50%),<br>> > %bb.6(37.50%)<br>> > <br>> > %8:an32_rn_pairs =...<br>> > cmp ...<br>> > brr_cond %bb.7<br>> > brr_uncond %bb.6<br>> > <br>> > [...]<br>> > <br>> > bb.7.cond.end38:<br>> > ; predecessors: %bb.4, %bb.6, %bb.2<br>> > successors: %bb.8(0x00000001), %bb.9(0x7fffffff); %bb.8(0.00%),<br>> > %bb.9(100.00%)<br>> > <br>> > %2:an32_0_7 = PHI %8:an32_rn_pairs, %bb.-1, %11:an32_rn_pairs,<br>> > %bb.4,<br>> > %0:an32_rn_pairs, %bb.6<br>> > cmps ...<br>> > brr_cond %bb.8<br>> > <br>> > <br>> > So it looks like it's failed to update the PHI operand for %bb.5 in<br>> > %2<br>> > properly leaving it now with %bb.-1?<br>> > <br>> > Any idea?<br>> > <br>> > /Mikael<br>> > <br>> > On 2019-08-17 16:37, Kang Zhang via llvm-commits wrote:<br>> > > Author: zhangkang<br>> > > Date: Sat Aug 17 07:37:05 2019<br>> > > New Revision: 369191<br>> > > <br>> > > URL: <br>> > > </font></tt><a href="https://protect2.fireeye.com/url?k=6f40a04c-3393b024-6f40e0d7-8691b328a8b8-90715b879d6c2a6b&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D369191%26view%3Drev"><tt><u><font size="2" color="#0000FF">https://protect2.fireeye.com/url?k=6f40a04c-3393b024-6f40e0d7-8691b328a8b8-90715b879d6c2a6b&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D369191%26view%3Drev</font></u></tt></a><tt><font size="2"> <br>> > > Log:<br>> > > [CodeGen] Do the Simple Early Return in block-placement pass to<br>> > > optimize the blocks<br>> > > <br>> > > Summary:<br>> > > <br>> > > Fix a bug of preducessors.<br>> > > <br>> > > In `block-placement` pass, it will create some patterns for<br>> > > unconditional we can do the simple early retrun.<br>> > > But the `early-ret` pass is before `block-placement`, we don't<br>> > > want to run it again.<br>> > > This patch is to do the simple early return to optimize the<br>> > > blocks at the last of `block-placement`.<br>> > > <br>> > > Reviewed By: efriedma<br>> > > <br>> > > Differential Revision: <br>> > > </font></tt><a href="https://protect2.fireeye.com/url?k=af91b373-f342a31b-af91f3e8-8691b328a8b8-a356f6b65b5e7513&q=1&u=https%3A%2F%2Freviews.llvm.org%2FD63972"><tt><u><font size="2" color="#0000FF">https://protect2.fireeye.com/url?k=af91b373-f342a31b-af91f3e8-8691b328a8b8-a356f6b65b5e7513&q=1&u=https%3A%2F%2Freviews.llvm.org%2FD63972</font></u></tt></a><tt><font size="2"> <br>> > > <br>> > > Modified:<br>> > > llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp<br>> > > llvm/trunk/test/CodeGen/PowerPC/block-placement.mir<br>> > > <br>> > > Modified: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp<br>> > > URL: <br>> > > </font></tt><a href="https://protect2.fireeye.com/url?k=44a01ed0-18730eb8-44a05e4b-8691b328a8b8-590610f76548b1fa&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Flib%2FCodeGen%2FMachineBlockPlacement.cpp%3Frev%3D369191%26r1%3D369190%26r2%3D369191%26view%3Ddiff"><tt><u><font size="2" color="#0000FF">https://protect2.fireeye.com/url?k=44a01ed0-18730eb8-44a05e4b-8691b328a8b8-590610f76548b1fa&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Flib%2FCodeGen%2FMachineBlockPlacement.cpp%3Frev%3D369191%26r1%3D369190%26r2%3D369191%26view%3Ddiff</font></u></tt></a><tt><font size="2"> <br>> > > =================================================================<br>> > > =============<br>> > > --- llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp (original)<br>> > > +++ llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp Sat Aug 17<br>> > > 07:37:05 2019<br>> > > @@ -2711,6 +2711,7 @@ void MachineBlockPlacement::optimizeBran<br>> > > // cannot because all branches may not be analyzable.<br>> > > // E.g., the target may be able to remove an unconditional<br>> > > branch to<br>> > > // a fallthrough when it occurs after predicated<br>> > > terminators.<br>> > > + SmallVector<MachineBasicBlock*, 4> EmptyBB;<br>> > > for (MachineBasicBlock *ChainBB : FunctionChain) {<br>> > > Cond.clear();<br>> > > MachineBasicBlock *TBB = nullptr, *FBB = nullptr; // For<br>> > > AnalyzeBranch.<br>> > > @@ -2730,9 +2731,45 @@ void MachineBlockPlacement::optimizeBran<br>> > > TII->removeBranch(*ChainBB);<br>> > > TII->insertBranch(*ChainBB, FBB, TBB, Cond, dl);<br>> > > ChainBB->updateTerminator();<br>> > > + } else if (Cond.empty() && TBB && ChainBB != TBB && !TBB-<br>> > > >empty() &&<br>> > > + !TBB->canFallThrough()) {<br>> > > + // When ChainBB is unconditional branch to the TBB, and<br>> > > TBB has no<br>> > > + // fallthrough predecessor and fallthrough successor,<br>> > > try to merge<br>> > > + // ChainBB and TBB. This is legal under the one of<br>> > > following conditions:<br>> > > + // 1. ChainBB is empty except for an unconditional<br>> > > branch.<br>> > > + // 2. TBB has only one predecessor.<br>> > > + MachineFunction::iterator I(TBB);<br>> > > + if (((TBB == &*F->begin()) || !std::prev(I)-<br>> > > >canFallThrough()) &&<br>> > > + (TailDup.isSimpleBB(ChainBB) || (TBB->pred_size()<br>> > > == 1))) {<br>> > > + TII->removeBranch(*ChainBB);<br>> > > + ChainBB->removeSuccessor(TBB);<br>> > > +<br>> > > + // Update the CFG.<br>> > > + while (!TBB->pred_empty()) {<br>> > > + MachineBasicBlock *Pred = *(TBB->pred_end() - 1);<br>> > > + Pred->ReplaceUsesOfBlockWith(TBB, ChainBB);<br>> > > + }<br>> > > +<br>> > > + while (!TBB->succ_empty()) {<br>> > > + MachineBasicBlock *Succ = *(TBB->succ_end() - 1);<br>> > > + ChainBB->addSuccessor(Succ, MBPI-<br>> > > >getEdgeProbability(TBB, Succ));<br>> > > + TBB->removeSuccessor(Succ);<br>> > > + }<br>> > > +<br>> > > + // Move all the instructions of TBB to ChainBB.<br>> > > + ChainBB->splice(ChainBB->end(), TBB, TBB->begin(),<br>> > > TBB->end());<br>> > > + EmptyBB.push_back(TBB);<br>> > > + }<br>> > > }<br>> > > }<br>> > > }<br>> > > +<br>> > > + for (auto BB: EmptyBB) {<br>> > > + MLI->removeBlock(BB);<br>> > > + FunctionChain.remove(BB);<br>> > > + BlockToChain.erase(BB);<br>> > > + F->erase(BB);<br>> > > + }<br>> > > }<br>> > > <br>> > > void MachineBlockPlacement::alignBlocks() {<br>> > > @@ -3052,6 +3089,9 @@ bool MachineBlockPlacement::runOnMachine<br>> > > }<br>> > > }<br>> > > <br>> > > + // optimizeBranches() may change the blocks, but we haven't<br>> > > updated the<br>> > > + // post-dominator tree. Because the post-dominator tree won't<br>> > > be used after<br>> > > + // this function and this pass don't preserve the post-<br>> > > dominator tree.<br>> > > optimizeBranches();<br>> > > alignBlocks();<br>> > > <br>> > > <br>> > > Modified: llvm/trunk/test/CodeGen/PowerPC/block-placement.mir<br>> > > URL: <br>> > > </font></tt><a href="https://protect2.fireeye.com/url?k=c2447a57-9e976a3f-c2443acc-8691b328a8b8-7cd3fcb165ec6683&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Ftest%2FCodeGen%2FPowerPC%2Fblock-placement.mir%3Frev%3D369191%26r1%3D369190%26r2%3D369191%26view%3Ddiff"><tt><u><font size="2" color="#0000FF">https://protect2.fireeye.com/url?k=c2447a57-9e976a3f-c2443acc-8691b328a8b8-7cd3fcb165ec6683&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Ftest%2FCodeGen%2FPowerPC%2Fblock-placement.mir%3Frev%3D369191%26r1%3D369190%26r2%3D369191%26view%3Ddiff</font></u></tt></a><tt><font size="2"> <br>> > > =================================================================<br>> > > =============<br>> > > --- llvm/trunk/test/CodeGen/PowerPC/block-placement.mir<br>> > > (original)<br>> > > +++ llvm/trunk/test/CodeGen/PowerPC/block-placement.mir Sat Aug<br>> > > 17 07:37:05 2019<br>> > > @@ -209,14 +209,10 @@ body: |<br>> > > BLR8 implicit $lr8, implicit $rm, implicit killed $x3<br>> > > <br>> > > ; CHECK: bb.5.if.else.i:<br>> > > - ; CHECK: successors: %bb.11(0x80000000)<br>> > > - ; CHECK: B %bb.11<br>> > > + ; CHECK-NEXT: renamable $x3 = LI8 1<br>> > > + ; CHECK-NEXT: BLR8 implicit $lr8, implicit $rm, implicit<br>> > > killed $x3<br>> > > <br>> > > ; CHECK: bb.8.while.body.i (align 4):<br>> > > - ; CHECK: successors: %bb.11(0x04000000),<br>> > > %bb.9(0x7c000000)<br>> > > - ; CHECK: BCC 76, killed renamable $cr0, %bb.11<br>> > > -<br>> > > - ; CHECK: bb.11:<br>> > > - ; CHECK: renamable $x3 = LI8 1<br>> > > - ; CHECK-NEXT: BLR8 implicit $lr8, implicit $rm, implicit<br>> > > killed $x3<br>> > > + ; CHECK: successors: %bb.5(0x04000000),<br>> > > %bb.9(0x7c000000)<br>> > > + ; CHECK: BCC 76, killed renamable $cr0, %bb.5<br>> > > ...<br>> > > <br>> > > <br>> > > _______________________________________________<br>> > > llvm-commits mailing list<br>> > > </font></tt><a href="mailto:llvm-commits@lists.llvm.org"><tt><u><font size="2" color="#0000FF">llvm-commits@lists.llvm.org</font></u></tt></a><tt><font size="2"><br>> > > </font></tt><tt><u><font size="2" color="#0000FF"><br></font></u></tt><a href="https://protect2.fireeye.com/url?k=b7a41100-eb770168-b7a4519b-8691b328a8b8-00280ce3331de496&q=1&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-commits"><tt><u><font size="2" color="#0000FF">https://protect2.fireeye.com/url?k=b7a41100-eb770168-b7a4519b-8691b328a8b8-00280ce3331de496&q=1&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-commits</font></u></tt></a><tt><font size="2"> <br>> > > <br>> > <br>> > _______________________________________________<br>> > llvm-commits mailing list<br>> > </font></tt><a href="mailto:llvm-commits@lists.llvm.org"><tt><u><font size="2" color="#0000FF">llvm-commits@lists.llvm.org</font></u></tt></a><tt><font size="2"><br>> > </font></tt><tt><u><font size="2" color="#0000FF"><br></font></u></tt><a href="https://protect2.fireeye.com/url?k=5d94a315-011e68e1-5d94e38e-86cd58c48020-a1db560ed36cea73&q=1&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-commits"><tt><u><font size="2" color="#0000FF">https://protect2.fireeye.com/url?k=5d94a315-011e68e1-5d94e38e-86cd58c48020-a1db560ed36cea73&q=1&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-commits</font></u></tt></a><tt><font size="2"> <br>> <br>> <br>_______________________________________________<br>llvm-commits mailing list</font></tt><tt><u><font size="2" color="#0000FF"><br></font></u></tt><a href="mailto:llvm-commits@lists.llvm.org"><tt><u><font size="2" color="#0000FF">llvm-commits@lists.llvm.org</font></u></tt></a><tt><u><font size="2" color="#0000FF"><br></font></u></tt><a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"><tt><u><font size="2" color="#0000FF">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</font></u></tt></a><tt><font size="2"> <br></font></tt><br><br><br></ul></ul><br><br><BR>
</body></html>