[llvm-dev] Remove zext-unfolding from InstCombine

[Matthias Reisinger via llvm-dev]

> 
> Am 05.08.2016 um 02:14 schrieb Sanjay Patel <spatel at rotateright.com>:
> 
> This is a general problem for any pattern matching of commutative operators. Search for "matchSelectFromAndOr" to see a particularly bad example. :)
> 
> In theory, we should be able to use "value complexity" in this situation, but it's broken:
> https://llvm.org/bugs/show_bug.cgi?id=28296
> 
> So either we need to fix that or we'll have to include extra patterns to match the commuted possibilities.

Thank you for that pointer! I had a look a this `getComplexity` function and it would obviously be a cleaner solution to use this complexity based approach. I think it would really be worth to fix this bug, also in order to simplify this `matchSelectFromAndOr` :) So I’ll first try to come up with a solution for that, if feasible. Otherwise, as a start, we might have to go with a manual matching of the commuted patterns.

>  
> 
> > We may need to answer this question first though:
> >  define i8 @notLowBit(i8 %a) {
> >   %lowBit = and i8 %a, 1
> >   %not = xor i8 %lowBit, 1
> >   ret i8 %not
> > }
> >
> > Should this be canonicalized to:
> > define i8 @notLowBit(i8 %a) {
> >   %not = xor i8 %a, -1
> >   %lowBit = and i8 %not, 1
> >   ret i8 %lowBit
> > }
> >
> > ...because xor with -1 is the canonical 'not' operation?
> 
> Do we need such a canonicalization in the presence of the above patterns?
> 
> This one is interesting. When I wrote the example, I didn't realize that we actually have the inverse canonicalization: the 2nd case is always transformed to the 1st because it reduces the demanded bits of the constant. However, there is a FIXME comment about that in SimplifyDemandedUseBits:
> 
>     // FIXME: for XOR, we prefer to force bits to 1 if they will make a -1.
> 
> So either we fix that and adjust the patterns that you are about to add or…

Yes, I also realized then that there exists this inverse transform. But for now, if I’m not mistaken, that doesn’t seem to be a problem because currently the new patterns don’t seem to be in need of such a transform.

> Welcome to InstCombine. :)

Thank you :)

Best,
Matthias