<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 6, 2016 at 12:24 PM, Aditya Kumar <span dir="ltr"><<a href="mailto:hiraditya@msn.com" target="_blank">hiraditya@msn.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hiraditya added a comment.<br>
<span><br>
In <a href="http://reviews.llvm.org/D18830#393400" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18830#393400</a>, @joker.eph wrote:<br>
<br>
> 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")<br>
<br>
<br>
</span>It seems VN.lookupOrAdd only swaps operands based on value numbers. </blockquote><div><br></div><div>At one point, this was necessary for correctness in some of it's other algorithms (don't ask).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Preferring constants/arguments on RHS seems unrelated to that, although I'm not sure this preference is useful in anyway.</blockquote><div><br></div><div>Yes, the constant/argument preference is in propagateEquality, not lookupOrAdd.<br></div><div>In GVN, it originally existed because it wanted a completely and totally canonical form.<br></div><div><br></div><div>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).<br></div><div><br></div><div>In practice, the IR should be canonicalized such that neither GVN nor NewGVN have to do this for any reason.</div><div>It's probably more useful to bootstrap with an assert that the constant is on the RHS and fix whatever is not doing that :)<br><br></div><div><br></div></div></div></div>