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

[Daniel Berlin via llvm-commits]

No I mean in the sense that the canonical form for the IR should be defined
so that we never have to swap.

On Wed, Apr 27, 2016, 4:28 PM Mehdi Amini <mehdi.amini at apple.com> wrote:

> On Apr 7, 2016, at 8:17 AM, Daniel Berlin <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> wrote:
>
>> hiraditya added a comment.
>>
>> In 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
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160428/0b48ed89/attachment.html>