No I mean in the sense that the canonical form for the IR should be defined so that we never have to swap.<br><br><div class="gmail_quote"><div dir="ltr">On Wed, Apr 27, 2016, 4:28 PM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com">mehdi.amini@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><blockquote type="cite"><div>On Apr 7, 2016, at 8:17 AM, Daniel Berlin <<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>> wrote:</div><br><div><div dir="ltr">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"</div></div></blockquote><div><br></div></div></div><div style="word-wrap:break-word"><div><div>"if we do, we should fix that"": isn't it what the code actually does a few lines below?</div><div><br></div><div>I don't mind at all this patch going in, the motivation for it is just unclear to me (it is NFC right?).</div></div></div><div style="word-wrap:break-word"><div><div><br></div><div>-- </div><div>Mehdi</div></div></div><div style="word-wrap:break-word"><div><div><br></div><br><blockquote type="cite"><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Apr 7, 2016 at 7:46 AM, 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#394173" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18830#394173</a>, @chandlerc wrote:<br>
<br>
</span><span>> Please don't avoid extra work in this way IMO, unless you have a very significant compile time hit due to this.<br>
><br>
> 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.<br>
><br>
</span><span>> My two cents. If Danny, Chad, or others working more on GVN disagree, they can happily override me. =D<br>
<br>
<br>
</span>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.<br>
<div><div><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D18830" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18830</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div>
</div></blockquote></div></div></blockquote></div>