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

[Daniel Berlin via llvm-commits]

On Wed, Apr 6, 2016 at 12:24 PM, Aditya Kumar <hiraditya at msn.com> wrote:

> hiraditya added a comment.
>
> In http://reviews.llvm.org/D18830#393400, @joker.eph wrote:
>
> > Oh then it is not even clear if it is worth it (avoid extra work here is
> doing an extra comparison to avoid a "pointer swap")
>
>
> It seems VN.lookupOrAdd only swaps operands based on value numbers.


At one point, this was necessary for correctness in some of it's other
algorithms (don't ask).

Preferring constants/arguments on RHS seems unrelated to that, although I'm
> not sure this preference is useful in anyway.


Yes, the constant/argument preference is in propagateEquality, not
lookupOrAdd.
In GVN, it originally existed because it wanted a completely and totally
canonical form.

NewGVN has the same preference, but there it is done to avoid having an
order-independent hash function (IE it doesn't ever take advantage of this
fact).

In practice, the IR should be canonicalized such that neither GVN nor
NewGVN have to do this for any reason.
It's probably more useful to bootstrap with an assert that the constant is
on the RHS and fix whatever is not doing that :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160407/7b41467c/attachment.html>