<div dir="ltr">I don't see any downside for the other users of shouldChangeType(), so I've cleaned up the patch and posted it for review:<br><a href="https://reviews.llvm.org/D29336">https://reviews.llvm.org/D29336</a><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 31, 2017 at 9:02 AM, Björn Steinbrink <span dir="ltr"><<a href="mailto:bsteinbr@gmail.com" target="_blank">bsteinbr@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><span class="">2017-01-31 16:59 GMT+01:00 Sanjay Patel <span dir="ltr"><<a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Tue, Jan 31, 2017 at 3:09 AM, Björn Steinbrink <span dir="ltr"><<a href="mailto:bsteinbr@gmail.com" target="_blank">bsteinbr@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Sanjay,<br><div><div class="gmail_extra"><br><div class="gmail_quote"><span class="m_3640621186349630882m_8407160113566485763m_9015473828703941006gmail-">2017-01-30 22:22 GMT+01:00 Sanjay Patel <span dir="ltr"><<a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">My minimal patch idea is to ease the restriction in ShouldChangeType because i1 is special. I tried the patch below, and it works on the example...and nothing in 'make check' failed. :)<br></div></blockquote><div><br></div></span><div>Yeah, that would work for me as well, I just wasn't sure about the implications that has. Simply making FoldPHIArgOpIntoPHI act like FoldPHIArgZextsIntoPHI seemed like the safer option to me, but I wanted feedback on it before creating a PR. Do you want to go ahead with that minimal approach and create a PR yourself?</div></div></div></div></div></blockquote><div><br></div></span><div>Your idea probably has the minimal impact while still fixing your problem case...although the relationships between FoldPHIArgOpIntoPHI / FoldPHIArgZextsIntoPHI / FoldOpIntoPhi are confusing.<br><br>I think a patch for ShouldChangeType makes sense on its own (and makes any direct changes to the FoldPHI* functions unnecessary for your example). I'll try some more experiments with that patch to look for unintended consequences. <br></div></div></div></div></blockquote><div><br></div></span><div>OK, I'll wait for your results then<br> <br></div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>When you say "PR", do you mean a bugzilla report or a patch proposal on Phab?<br></div></div></div></div></blockquote><div><br></div></span><div>I meant a patch proposal on Phab, I'm too used to GitHub terminology by now, sorry.<br><br></div><div>Cheers,<br></div><div>Björn<br><br></div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div><div class="m_3640621186349630882h5"><div><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div class="gmail_extra"><div class="gmail_quote"><div><span class="m_3640621186349630882m_8407160113566485763m_9015473828703941006gmail-HOEnZb"><font color="#888888"><br><br></font></span></div><span class="m_3640621186349630882m_8407160113566485763m_9015473828703941006gmail-HOEnZb"><font color="#888888"><div>Björn<br></div></font></span><div><div class="m_3640621186349630882m_8407160113566485763m_9015473828703941006gmail-h5"><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Index: lib/Transforms/InstCombine/Ins<wbr>tructionCombining.cpp<br>==============================<wbr>==============================<wbr>=======<br>--- lib/Transforms/InstCombine/Ins<wbr>tructionCombining.cpp    (revision 293485)<br>+++ lib/Transforms/InstCombine/Ins<wbr>tructionCombining.cpp    (working copy)<br>@@ -88,12 +88,12 @@<br> <br> /// Return true if it is desirable to convert an integer computation from a<br> /// given bit width to a new bit width.<br>-/// We don't want to convert from a legal to an illegal type for example or from<br>-/// a smaller to a larger illegal type.<br>+/// We don't want to convert from a legal to an illegal type or from a smaller<br>+/// to a larger illegal type. Width of '1' is always treated as a legal type.<br> bool InstCombiner::ShouldChangeType<wbr>(unsigned FromWidth,<br>                              <wbr>       unsigned ToWidth) const {<br>-  bool FromLegal = DL.isLegalInteger(FromWidth);<br>-  bool ToLegal = DL.isLegalInteger(ToWidth);<br>+  bool FromLegal = FromWidth == 1 ? true : DL.isLegalInteger(FromWidth);<br>+  bool ToLegal = ToWidth == 1 ? true : DL.isLegalInteger(ToWidth);<br> <br>   // If this is a legal integer from type, and the result would be an illegal<br>   // type, don't do the transformation.<br>@@ -110,7 +110,7 @@<br> <br> /// Return true if it is desirable to convert a computation from 'From' to 'To'.<br> /// We don't want to convert from a legal to an illegal type for example or from<br>-/// a smaller to a larger illegal type.<br>+/// a smaller to a larger illegal type. i1 is always treated as a legal type.<br> bool InstCombiner::ShouldChangeType<wbr>(Type *From, Type *To) const {<br>   assert(From->isIntegerTy() && To->isIntegerTy());<br></div></blockquote></div></div></div></div></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>