<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">I don't have a problem with the patch. But is there a follow up patch to fix the lowering problem?<div><br></div><div>Looking back at the history. It mentioned this impacts a couple of small test cases:</div><div><br></div><div><div>int</div><div>blah(_Bool x, _Bool y) {</div><div> return (x - y) + 1;</div><div>}</div><div><br></div><div><div>int</div><div>blah2(bool x) {</div><div> return x ? 0 : -1;</div><div>}</div></div><div><br></div><div>BTW, David filed the bug report that motivated the patch.</div><div><br></div><div>Evan</div><div><br><div><div>On Jul 19, 2013, at 2:10 PM, Nick Lewycky <<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div dir="ltr">Hi Evan, this is about your r159230,<span class="Apple-converted-space"> </span><a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120625/145470.html">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120625/145470.html</a><span class="Apple-converted-space"> </span>. The canonicalization was added, then removed by you. In post-commit review, Eli asked you to pick one of the two forms to be canonical.<br><div><br></div><div>I have no problem with David landing this patch as is -- it looks like goodness to me, particularly the "C - zext(X)" transform -- but I don't want to mess you up. Could you please chime in?</div><div><br></div><div>Nick</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On 12 July 2013 16:03, David Majnemer<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div dir="ltr"><div>The attached patch essentially reverts r159230.</div><div><br></div><div>The reasons why are because:</div><div>1. It creates an asymmetry in the IR by hurting our ability to canonicalize things like C - zext(X) and C + zext(!X) into the same thing.</div><div>2. Adding it back does not break any tests and without it we still generate the inferior code mentioned in the commit log.</div><div><br></div><div>It seems like we should keep the canonicalization and fix (the apparently still broken problem) in the lowering stage.</div><div>The commit mentioned that some targets don't have a sext i1. This shouldn't be a concern at the IR level, should it?</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-- </div><div>David Majnemer</div></font></span></div><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></blockquote></div></div></div></blockquote></div><br></div></div></body></html>