<div dir="ltr">> <span style="font-size:13.1999998092651px;line-height:19.7999992370605px">Given what you are saying, you are also suggesting we are not</span><br style="font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">rematerializing addressing computations where it is cheaper to do so.</span><br style="font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">That seems pretty critical to good RA :P</span><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px"><br></span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">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><span style="font-size:13.1999998092651px;line-height:19.7999992370605px"><br></span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">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">dberlin@dberlin.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;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>