<html><head><style type='text/css'>p { margin: 0; }</style></head><body><div style='font-family: arial,helvetica,sans-serif; font-size: 10pt; color: #000000'><br><hr id="zwchr"><blockquote id="DWT27082" style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><b>From: </b>"Philip Reames" <listmail@philipreames.com><br><b>To: </b>"James Molloy" <james@jamesmolloy.co.uk>, "Daniel Berlin" <dberlin@dberlin.org>, "Lawrence" <lawrence@codeaurora.org><br><b>Cc: </b>"LLVM Developers Mailing List" <llvmdev@cs.uiuc.edu><br><b>Sent: </b>Wednesday, July 15, 2015 4:43:51 PM<br><b>Subject: </b>Re: [LLVMdev] Register pressure mechanism in PRE or Smarter rematerialization/split/spiller/coalescing ?<br><br>
<br>
<br>
<div class="moz-cite-prefix">On 07/15/2015 01:47 PM, James Molloy
wrote:<br>
</div>
<blockquote cite="mid:CALCTSA0MTSdU4oiwzM3n3WS519tKX8McSR4GVfWH3wodSCusgw@mail.gmail.com">
<div dir="ltr">> <span style="font-size: 13.2px; line-height: 19.8px;">Given
what you are saying, you are also suggesting we are not</span><br style="font-size: 13.2px; line-height: 19.8px;">
<span style="font-size: 13.2px; line-height: 19.8px;">rematerializing
addressing computations where it is cheaper to do so.</span><br style="font-size: 13.2px; line-height: 19.8px;">
<span style="font-size: 13.2px; line-height: 19.8px;">That
seems pretty critical to good RA :P</span>
<div><span style="font-size: 13.2px; line-height: 19.8px;"><br>
</span></div>
<div><span style="font-size: 13.2px; line-height: 19.8px;">Yep,
about 5 months ago I had a conversation about this too... it
may even be the one you're referencing. Our remat is really
conservative - it only rematerializes values that have zero
input operands (move immediate only, essentially).</span></div>
</div>
</blockquote>
I thought we could remat explicitly invariant loads as well?<br></blockquote><br>We can. See TargetInstrInfo::isReallyTriviallyReMaterializableGeneric in lib/CodeGen/TargetInstrInfo.cpp -- What we can't do, for example, it rematerialize small sequences of instructions, such as those that form constants out of smaller immediates.<br><br> -Hal<br><br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;">
<blockquote cite="mid:CALCTSA0MTSdU4oiwzM3n3WS519tKX8McSR4GVfWH3wodSCusgw@mail.gmail.com">
<div dir="ltr">
<div><span style="font-size: 13.2px; line-height: 19.8px;"><br>
</span></div>
<div><span style="font-size: 13.2px; line-height: 19.8px;">James</span></div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Wed, 15 Jul 2015 at 21:28 Daniel Berlin <<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">IMHO, This
doesn't make a lot of sense to turn off this part on it's own.<br>
I would just use the enable-pre flag to turn off scalar PRE,
as it<br>
will cause the same issue in other cases as well.<br>
Is there some reason you aren't just doing that?<br>
I suspect if this is a performance win, that would be as well.<br>
<br>
Also note that you will have the same problem as
GVN/EarlyCSE/etc<br>
becomes smarter, as these are full redundancies being
eliminated (IE<br>
there is no insertion happening). It just happens that PRE
notices<br>
them and GVN doesn't, because GVN is dominator based and PRE
is not.<br>
A slightly smarter GVN/EarlyCSE would do the same thing.<br>
<br>
<br>
Given what you are saying, you are also suggesting we are not<br>
rematerializing addressing computations where it is cheaper to
do so.<br>
That seems pretty critical to good RA :P<br>
<br>
<br>
<br>
On Wed, Jul 15, 2015 at 11:51 AM, Lawrence <<a href="mailto:lawrence@codeaurora.org" target="_blank">lawrence@codeaurora.org</a>> wrote:<br>
> Hi, Daniel:<br>
><br>
> Thanks a lot for detailed background information, we are
willing to provide the right fix, however it will take time,
do you mind if you forward me the discussion you had 5 months
ago? I may not be able to access it since I only joined
ellvmdev list this week.<br>
><br>
> I did some performance measurement last night, some of
our critical benchmark degraded up to 30% with your patch, so
we have to turn it off for now at least.<br>
><br>
> I posted patch to add a debug option (off by default), so
we could turn it off with that option, could you please review
it and commit it for me if possible? I don't have commit
right yet, will ask soon.<br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11234&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=Mfk2qtn1LTDThVkh6-oGglNfMADXfJdty4_bhmuhMHA&m=zIdYS4bXbG63-LsNf28NXGKkrEdKPhY_mdzZrzA1eQE&s=6zQDYowITv9KrC3Glp-DVdhOmWxIQRFN5V8VIAdvnJQ&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11234</a><br>
><br>
> Thanks again.<br>
><br>
> Lawrence Hu<br>
><br>
><br>
> -----Original Message-----<br>
> From: Daniel Berlin [mailto:<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>]<br>
> Sent: Wednesday, July 15, 2015 7:48 AM<br>
> To: Lawrence<br>
> Cc: LLVM Developers Mailing List<br>
> Subject: Re: Register pressure mechanism in PRE or
Smarter rematerialization/split/spiller/coalescing ?<br>
><br>
> On Tue, Jul 14, 2015 at 11:43 PM, Lawrence <<a href="mailto:lawrence@codeaurora.org" target="_blank">lawrence@codeaurora.org</a>> wrote:<br>
>> I thought about a little bit more, I think adding
Register pressure control in your patch or PRE may be the only
choice.<br>
>><br>
>> Because at least for this case I am looking at, what
your patch did is created more relatively complex long live
range, rematerialization is not smart enough to undo your
change or at least without a lot of work, coalescing only
create even longer live range not shorter, Spiller can't help
since it's the Spiller created Spill/Reloads due to high
register pressure, Splitting can shorten the live ranges, but
I don't think it can handle your case without a lot of work.<br>
>><br>
><br>
> 1. As I mentioned, it simply fixes a bug in
implementation of one of the two PRE's LLVM has. It does not
change the PRE algorithm or add<br>
> anything to it. The code had a bug. I fixed the bug :P.
PRE is<br>
> *not even adding more code in this case*. The code is
already there.<br>
> All it is doing is inserting a phi node. If you
transformed your code to use memory, and reverted my patch,
you'd get the same result, because Load PRE will do the same
thing. It's what PRE does.<br>
><br>
> 2. GCC and other compilers have PRE's literally the same
thing my patch does (you are welcome to verify, i wrote GCC's
:P), and apparently are smart enough to handle this in RA. So
i'm going to suggest that it is, in fact, possible to do so,
and i'm going to further suggest that if we want to match
their performance, we need to be able to do the same. You
can't simply "turn down" any optimization that RA may have to
deal with. It usually doesn't work in practice.<br>
> This is one of the reasons good RA is so hard.<br>
><br>
> 3. As I also mentioned, register pressure heuristics in
PRE simply do not work. They've been tried. By many. With
little to no success.<br>
><br>
> PRE is too high in the stack of optimizations to estimate
register<br>
> pressure in any sane fashion. It's pretty much a fools
errand. You<br>
> can never tune it to do what you want. *Many* have
tried.<br>
><br>
> Your base level problem here is that all modern PRE
algorithms (except for min-cut PRE, as I mentioned), are based
on a notion of lifetime optimality. That is, they extend
lifetimes as minimally as possible to still eliminate a given
redundancy. Ours does the same.<br>
><br>
> However, this is not an entirely useful metric.
Optimizing for some other metric is what something like
min-cut PRE lets you do.<br>
> But even then, register pressure heuristics are almost
certainly not the answer.<br>
><br>
> 4. This was actually already discussed when the patch was
submitted, and the consensus was "we should just fix RA".
Feel free to look at the discussion 5 months ago.<br>
><br>
> I would suggest, if you want to fix this, you take the
approach that was discussed then.<br>
><br>
<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu" target="_blank">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" rel="noreferrer" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
</blockquote>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre>_______________________________________________<br>LLVM Developers mailing list<br><a class="moz-txt-link-abbreviated" href="mailto:LLVMdev@cs.uiuc.edu" target="_blank">LLVMdev@cs.uiuc.edu</a> <a class="moz-txt-link-freetext" href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br><a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br></pre>
</blockquote>
<br>
<br>_______________________________________________<br>LLVM Developers mailing list<br>LLVMdev@cs.uiuc.edu http://llvm.cs.uiuc.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev<br></blockquote><br><br><br>-- <br><div><span name="x"></span>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<span name="x"></span><br></div></div></body></html>