[PATCH] D18830: [llvm] GVN.cpp: Do not swap when both LHS and RHS are arguments.

[Mehdi Amini via llvm-commits]

> On Apr 7, 2016, at 8:17 AM, Daniel Berlin <dberlin at dberlin.org <mailto:dberlin at dberlin.org>> wrote:
> 
> I think his point is more the same one i made: "we shouldn't have to swap at all, and if we do, we should fix that"

"if we do, we should fix that"": isn't it what the code actually does a few lines below?

I don't mind at all this patch going in, the motivation for it is just unclear to me (it is NFC right?).

-- 
Mehdi


> 
> On Thu, Apr 7, 2016 at 7:46 AM, Aditya Kumar <hiraditya at msn.com <mailto:hiraditya at msn.com>> wrote:
> hiraditya added a comment.
> 
> In http://reviews.llvm.org/D18830#394173 <http://reviews.llvm.org/D18830#394173>, @chandlerc wrote:
> 
> > Please don't avoid extra work in this way IMO, unless you have a very significant compile time hit due to this.
> >
> > We have a pervasive reliance on canonicalizing constants to the RHS of binary operations so those should be exceedingly rare anyways. We shouldn't fight that, we should run with it.
> >
> > My two cents. If Danny, Chad, or others working more on GVN disagree, they can happily override me. =D
> 
> 
> Thanks for the feedback. This patch will not change the order of preference (RHS should keep Constants/Arguments). it only prevents swap when both LHS and RHS are Arguments.
> 
> 
> Repository:
>   rL LLVM
> 
> http://reviews.llvm.org/D18830 <http://reviews.llvm.org/D18830>
> 
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160427/ae8b165c/attachment.html>