<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">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_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><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><br>When you say "PR", do you mean a bugzilla report or a patch proposal on Phab?<br></div><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_9015473828703941006gmail-HOEnZb"><font color="#888888"><br><br></font></span></div><span class="m_9015473828703941006gmail-HOEnZb"><font color="#888888"><div>Björn<br></div></font></span><div><div class="m_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><br></div></div>