<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<div class="moz-cite-prefix">On 08/31/2017 01:09 PM, Lei Huang
wrote:<br>
</div>
<blockquote
cite="mid:OFC2F97C74.61A4F8AA-ON0025818D.005F8BA0-0025818D.0063B777@notes.na.collabserv.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<div class="socmaildefaultfont" dir="ltr"
style="font-family:Arial, Helvetica,
sans-serif;font-size:10.5pt">
<div dir="ltr">Hi all,</div>
<div dir="ltr"> </div>
<div dir="ltr">Does anyone have any objections to me committing
this patch back with it turned off by default?</div>
</div>
</blockquote>
<br>
I think that's fine. Then you can post a patch to fix this bug.<br>
<br>
-Hal<br>
<br>
<blockquote
cite="mid:OFC2F97C74.61A4F8AA-ON0025818D.005F8BA0-0025818D.0063B777@notes.na.collabserv.com"
type="cite">
<div class="socmaildefaultfont" dir="ltr"
style="font-family:Arial, Helvetica,
sans-serif;font-size:10.5pt">
<div dir="ltr"> </div>
<div dir="ltr">
<div class="socmaildefaultfont" dir="ltr"
style="font-family:Arial, Helvetica,
sans-serif;font-size:10.5pt">
<div class="socmaildefaultfont" dir="ltr"
style="font-family:Arial, Helvetica,
sans-serif;font-size:10.5pt">
<div class="socmaildefaultfont" dir="ltr"
style="font-family:Arial, Helvetica,
sans-serif;font-size:10.5pt">
<div dir="ltr"><br>
Regards,<br>
Lei Huang</div>
<div dir="ltr"> </div>
<div dir="ltr"> </div>
<div dir="ltr">LLVM Development on POWER</div>
<div dir="ltr">Internal mail: C2/YGK/8200/MKM<br>
Phone: (905) 413-4419<br>
TieLine: 969-4419<br>
E-mail: <a class="moz-txt-link-abbreviated" href="mailto:lei@ca.ibm.com">lei@ca.ibm.com</a></div>
</div>
</div>
</div>
</div>
<div dir="ltr"> </div>
<div dir="ltr"> </div>
<blockquote data-history-content-modified="1"
data-history-expanded="1" dir="ltr" style="border-left:solid
#aaaaaa 2px; margin-left:5px; padding-left:5px; direction:ltr;
margin-right:0px">----- Original message -----<br>
From: Eric Christopher <a class="moz-txt-link-rfc2396E" href="mailto:echristo@gmail.com"><echristo@gmail.com></a><br>
To: <a class="moz-txt-link-abbreviated" href="mailto:reviews+D28249+public+34df2467c7dab1c5@reviews.llvm.org">reviews+D28249+public+34df2467c7dab1c5@reviews.llvm.org</a>,
<a class="moz-txt-link-abbreviated" href="mailto:lei@ca.ibm.com">lei@ca.ibm.com</a>, <a class="moz-txt-link-abbreviated" href="mailto:carrot@google.com">carrot@google.com</a>, <a class="moz-txt-link-abbreviated" href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>,
<a class="moz-txt-link-abbreviated" href="mailto:nemanja.i.ibm@gmail.com">nemanja.i.ibm@gmail.com</a><br>
Cc: <a class="moz-txt-link-abbreviated" href="mailto:kyle+llvm@iteratee.net">kyle+llvm@iteratee.net</a>, <a class="moz-txt-link-abbreviated" href="mailto:matze@braunis.de">matze@braunis.de</a>,
<a class="moz-txt-link-abbreviated" href="mailto:hiraditya@msn.com">hiraditya@msn.com</a>, <a class="moz-txt-link-abbreviated" href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>, <a class="moz-txt-link-abbreviated" href="mailto:syzaara@ca.ibm.com">syzaara@ca.ibm.com</a>,
<a class="moz-txt-link-abbreviated" href="mailto:sfertile@ca.ibm.com">sfertile@ca.ibm.com</a>, <a class="moz-txt-link-abbreviated" href="mailto:jtony@ca.ibm.com">jtony@ca.ibm.com</a>, <a class="moz-txt-link-abbreviated" href="mailto:amara.emerson@arm.com">amara.emerson@arm.com</a>,
<a class="moz-txt-link-abbreviated" href="mailto:joker.eph@gmail.com">joker.eph@gmail.com</a>, <a class="moz-txt-link-abbreviated" href="mailto:mgorny@gentoo.org">mgorny@gentoo.org</a>,
<a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>, Daniel Jasper
<a class="moz-txt-link-rfc2396E" href="mailto:djasper@google.com"><djasper@google.com></a><br>
Subject: Re: [PATCH] D28249: Improve scheduling with branch
coalescing<br>
Date: Thu, Aug 31, 2017 2:34 AM<br>
<div dir="ltr">Hi All,
<div> </div>
<div>Right now since we have a failing testcase internally
would anyone mind either:</div>
<div> </div>
<div>a) reverting the patch,</div>
<div>b) turning off the pass temporarily</div>
<div> </div>
<div>while we fix the issues or work up a more public
testcase?</div>
<div> </div>
<div> </div>
<div>
<div dir="ltr">On Wed, Aug 30, 2017 at 4:33 PM Hal Finkel
via Phabricator <<a moz-do-not-send="true"
href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>>
wrote:</div>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">hfinkel added inline comments.<br>
<br>
<br>
================<br>
Comment at:
llvm/trunk/lib/CodeGen/BranchCoalescing.cpp:340<br>
+ MachineInstr *Op2Def =
MRI->getVRegDef(Op2.getReg());<br>
+ if (TII->produceSameValue(*Op1Def, *Op2Def,
MRI)) {<br>
+ DEBUG(dbgs() << "Op1Def: " <<
*Op1Def << " and " << *Op2Def<br>
----------------<br>
iteratee wrote:<br>
> This line isn't correct. I have a case where
instructions that "produce the same value" are
different. The relevant sequence is:<br>
> ```<br>
> BB#16:<br>
> ...<br>
> BCTRL8_LDinto_toc<br>
> ...<br>
> %vreg140<def> = COPY %CR0GT;
CRBITRC:%vreg140<br>
> %vreg141<def> = LXSDX %vreg138,
%vreg129, %RM<imp-use>;
mem:LD8[%134](dereferenceable) F8RC:%vreg141
G8RC_and_G8RC_NOX0:%vreg138 G8RC:%vreg129<br>
> %vreg142<def> = XXLXORdpz;
F8RC:%vreg142<br>
> BC %vreg140, <BB#73>;
CRBITRC:%vreg140<br>
><br>
> BB#72: derived from LLVM BB %114<br>
> Predecessors according to CFG: BB#16<br>
> Successors according to CFG: BB#73(?%)<br>
><br>
> BB#73: derived from LLVM BB %114<br>
> Predecessors according to CFG: BB#16 BB#72<br>
> %vreg143<def> = PHI %vreg142,
<BB#72>, %vreg141, <BB#16>;
F8RC:%vreg143,%vreg142,%vreg141<br>
> ...<br>
> BCTRL8_LDinto_toc<br>
> ...<br>
> %vreg149<def> = COPY %CR0GT;
CRBITRC:%vreg149<br>
> %vreg150<def> = LXSDX %vreg138,
%vreg129, %RM<imp-use>;
mem:LD8[%134](dereferenceable) F8RC:%vreg150
G8RC_and_G8RC_NOX0:%vreg138 G8RC:%vreg129<br>
> BC %vreg149, <BB#75>;
CRBITRC:%vreg149<br>
> Successors according to CFG: BB#74(?%)
BB#75(?%)<br>
><br>
> BB#74: derived from LLVM BB %114<br>
> Predecessors according to CFG: BB#73<br>
> Successors according to CFG: BB#75(?%)<br>
><br>
> BB#75: derived from LLVM BB %114<br>
> Predecessors according to CFG: BB#73 BB#74<br>
> %vreg151<def> = PHI %vreg142,
<BB#74>, %vreg150, <BB#73>;
F8RC:%vreg151,%vreg142,%vreg150<br>
> ```<br>
><br>
><br>
> The debug output produces:<br>
> ```<br>
> Valid Candidate<br>
> Op1: 1024<br>
> Op2: 1024<br>
> Op1 and Op2 are identical!<br>
> Op1: %vreg140<br>
> Op2: %vreg149<br>
> Op1Def: %vreg140<def> = COPY %CR0GT;
CRBITRC:%vreg140<br>
> and %vreg149<def> = COPY %CR0GT;
CRBITRC:%vreg149<br>
> produce the same value!<br>
> ```<br>
><br>
> While it would be safe to CSE those crmoves, what
definitely cannot occur is to assume that the value of
CR0GT has not changed between the 2 instructions.<br>
Indeed; I think that we need to filter out instructions
with physical-register uses (even if identical). We
might be able to be a little smarter about it in a
similar way to MachineLICM (the other uses of this
function), which does this:<br>
<br>
// Don't hoist an instruction that uses or defines a
physical register.<br>
if (TargetRegisterInfo::isPhysicalRegister(Reg)) {<br>
if (MO.isUse()) {<br>
// If the physreg has no defs anywhere, it's
just an ambient register<br>
// and we can freely move its uses.
Alternatively, if it's allocatable,<br>
// it could get allocated to something with a
def during allocation.<br>
// However, if the physreg is known to always be
caller saved/restored<br>
// then this use is safe to hoist.<br>
if (!MRI->isConstantPhysReg(Reg) &&<br>
!(TRI->isCallerPreservedPhysReg(Reg,
*I.getParent()->getParent())))<br>
return false;<br>
// Otherwise it's safe to move.<br>
continue;<br>
} else if (!MO.isDead()) {<br>
// A def that isn't dead. We can't move it.<br>
return false;<br>
...<br>
<br>
<br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<a moz-do-not-send="true"
href="https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D28249&d=DwMFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=wcuzqlo__GgcNlt1_YAUPg&m=tHTjrXS2HNkfTEF9LX-FDD0fduLEs2MiEMJPQ2M53pQ&s=pF0fBNDL_Koy8xlMpGdnOFqnTdHitMwgjGYkLJ6tY_I&e="
rel="noreferrer" target="_blank">https://reviews.llvm.org/D28249</a><br>
<br>
<br>
</blockquote>
</div>
</div>
</blockquote>
<div dir="ltr"> </div>
</div>
<br>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</body>
</html>