<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Please, find reproducer in the comment to the original review:<br>
<a class="moz-txt-link-freetext" href="https://reviews.llvm.org/D63972#1647948">https://reviews.llvm.org/D63972#1647948</a><br>
<br>
regards,<br>
Fedor.<br>
<br>
<div class="moz-cite-prefix">On 8/27/19 6:08 PM, Jinsong Ji wrote:<br>
</div>
<blockquote type="cite"
cite="mid:OF032E8110.2F3AFA93-ON00258463.0052BA9E-85258463.0053296E@notes.na.collabserv.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<p><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><tt><font size="2"><a
href="https://reviews.llvm.org/rL370069"
moz-do-not-send="true">https://reviews.llvm.org/rL370069</a>.</font></tt><br>
<br>
<font size="2">Reproducer is appreciated. Thanks.<br>
</font><br>
<br>
<font size="2">Best,<br>
<br>
Jinsong Ji (纪金松), PhD.<br>
<br>
XL/LLVM on Power Compiler Development <br>
E-mail: <a class="moz-txt-link-abbreviated" href="mailto:jji@us.ibm.com">jji@us.ibm.com</a></font><br>
<br>
<img src="cid:part2.665FD1EE.70E3703F@azul.com" 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" class="" width="16" height="16" border="0"><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>
<br>
<font size="2" color="#5F5F5F">From: </font><font size="2">"Mikael
Holmén via llvm-commits" <a class="moz-txt-link-rfc2396E" href="mailto:llvm-commits@lists.llvm.org"><llvm-commits@lists.llvm.org></a></font><br>
<font size="2" color="#5F5F5F">To: </font><font size="2"><a class="moz-txt-link-rfc2396E" href="mailto:fedor.sergeev@azul.com">"fedor.sergeev@azul.com"</a>
<a class="moz-txt-link-rfc2396E" href="mailto:fedor.sergeev@azul.com"><fedor.sergeev@azul.com></a>, <a class="moz-txt-link-rfc2396E" href="mailto:shkzhang@cn.ibm.com">"shkzhang@cn.ibm.com"</a>
<a class="moz-txt-link-rfc2396E" href="mailto:shkzhang@cn.ibm.com"><shkzhang@cn.ibm.com></a></font><br>
<font size="2" color="#5F5F5F">Cc: </font><font size="2"><a class="moz-txt-link-rfc2396E" href="mailto:llvm-commits@lists.llvm.org">"llvm-commits@lists.llvm.org"</a>
<a class="moz-txt-link-rfc2396E" href="mailto:llvm-commits@lists.llvm.org"><llvm-commits@lists.llvm.org></a></font><br>
<font size="2" color="#5F5F5F">Date: </font><font size="2">08/27/2019
01:20 AM</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>
<font size="2" color="#5F5F5F">Sent by: </font><font size="2">"llvm-commits"
<a class="moz-txt-link-rfc2396E" href="mailto:llvm-commits-bounces@lists.llvm.org"><llvm-commits-bounces@lists.llvm.org></a></font><br>
</p>
<hr style="color:#8091A5; " width="100%" size="2"
noshade="noshade" align="left"><br>
<br>
<br>
<tt><font size="2">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><tt><font size="2"><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"
moz-do-not-send="true">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</a></font></tt><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><tt><font size="2"><a
href="https://protect2.fireeye.com/url?k=af91b373-f342a31b-af91f3e8-8691b328a8b8-a356f6b65b5e7513&q=1&u=https%3A%2F%2Freviews.llvm.org%2FD63972"
moz-do-not-send="true">https://protect2.fireeye.com/url?k=af91b373-f342a31b-af91f3e8-8691b328a8b8-a356f6b65b5e7513&q=1&u=https%3A%2F%2Freviews.llvm.org%2FD63972</a></font></tt><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><tt><font size="2"><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"
moz-do-not-send="true">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</a></font></tt><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><tt><font size="2"><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"
moz-do-not-send="true">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</a></font></tt><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>
> > > <a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> > > <br>
</font></tt><tt><font size="2"><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"
moz-do-not-send="true">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</a></font></tt><tt><font
size="2"> <br>
> > > <br>
> > <br>
> > _______________________________________________<br>
> > llvm-commits mailing list<br>
> > <a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> > <br>
</font></tt><tt><font size="2"><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"
moz-do-not-send="true">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</a></font></tt><tt><font
size="2"> <br>
> <br>
> <br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
</font></tt><tt><font size="2"><a
href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"
moz-do-not-send="true">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></font></tt><tt><font
size="2"> <br>
<br>
</font></tt><br>
<br>
<br>
</blockquote>
<br>
</body>
</html>