<div class="gmail_quote">On Fri, Mar 30, 2012 at 1:35 PM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com">clattner@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Mar 28, 2012, at 3:37 AM, Chandler Carruth wrote:<br>
> Currently, when converting an i8 loaded value to an i1 IR value, the canonical pattern is:<br>
><br>
> %x.1 = and i8 %x, 1<br>
> %x.2 = icmp ne i8 %x, 0<br>
><br>
> This seems really cumbersome. The natural way to represent this is:<br>
><br>
> %x.2 = trunc i8 %x to i1<br>
><br>
> But for some reason instcombine canonicalizes it to the first pattern. I feel like the second pattern is easier to reason about, easier to match in further peep-hole optimizations, easier to fold with surrounding zext operations, and easier to teach codegen to be clever about and eliminate altogether given range metadata. Thoughts? I've attached the trivial change to the code, but the real question is: do we want to do this? Why or why not?<br>

<br>
</div></div>This was added in r67635:<br>
<br>
"Make instcombine always canonicalize trunc x to i1 into an icmp(x&1).  This<br>
exposes the AND to other instcombine xforms and is more of what the code<br>
generator expects."<br>
<br>
The 'trunc to i1' is not legal for any target, and that exposing it to the IR optimizer allows us to simplify and eliminate the and.<br></blockquote><div><br></div><div>I had read the commit message, but it wasn't clear to me why we couldn't simply let the IR optimizer go to town on the trunc (as well as the and).</div>
<div><br></div><div>That said, the more I played with this, the less I liked it. The big reason is that at the IR level it is nice to canonicalize a single-bit test, regardless of *which* bit is being tested, into and+icmp. I kept adding more and more special cases, and still not catching all of the optimizations. So the original canonicalization makes more and more sense.</div>
<div><br></div><div>I think Rafael has a way to get codegen to still turn this into the right low-level instructions inside the dag combiner, so I'll refocus over there.</div><div><br></div><div>Sorry for the noise.</div>
<div>-Chandler</div></div>