[llvm-commits] [llvm] r147749 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp test/Transforms/InstCombine/sign-test-and-or.ll

[Benjamin Kramer]

On 09.01.2012, at 01:30, Dirk Steinke wrote:

> On 01/08/2012 11:50 PM, Eli Friedman wrote:
>> On Sun, Jan 8, 2012 at 10:32 AM, Benjamin Kramer
>> <benny.kra at googlemail.com>  wrote:
>>> Author: d0k
>>> Date: Sun Jan  8 12:32:24 2012
>>> New Revision: 147749
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=147749&view=rev
>>> Log:
>>> InstCombine: If we have a bit test and a sign test anded/ored together, merge the sign bit into the bit test.
> [snip]
>> Err, shouldn't this be "(X & C) != 0 | X < 0"?  It looks like the
>> actual transformation is correct, just the comment is wrong.

I fixed that in a followup, stupid copy&paste.

>> On a more general note, could this code be combined with the code that
>> transforms "(X&  C1) == 0&  (X&  C2) == 0 ->  (X&  (C1|C2)) == 0"?
>> It's essentially the same transformation; it's just that there's a
>> more canonical way of writing "(X&  SignBit) == 0".  Should be pretty
>> straightforward to write an "isAnyBitSetComparison()" helper.  (Could
>> also easily add some other potentially interesting transformations,
>> like "(X<u (1<<  C1))&  ((X&  C2) == 0)".)
>> 
>> -Eli
> 
> The code for "(X & C1) == 0 & (X & C2) == 0 -> (X & (C1|C2)) == 0" can actually combine a lot more bitpattern comparisons into a single icmp instruction (like (X & 5) == 0 & (X & 6) == 2 -> (X & 7) == 2).
> And it handles the inversed case as well:
> (X & 5) != 0 | (X & 6) != 2 -> (X & 7) != 2.
> 
> It currently only accepts instructions of the form A & B == C, or A & B != C. If the function to check this (foldLogOpOfMaskedICmpsHelper) was appropriately adapted, the code should also be able to handle
> other transformations, which are currently special cased,
> like
>  (icmp sgt A, -1) & (icmp sgt B, -1) --> (icmp sgt (A|B), -1)
> or
>  (trunc x) == C1 & (and x, CA) == C2 -> (and x, CA|CMAX) == C1|C2
> (I guess, a helper function for the output icmp would be in order, so that the code would not generate (icmp eq (X & signbit), 0), but (icmp sgt X,-1).)

I extended foldLogOpOfMaskedICmpsHelper to handle (icmp sgt X, -1) and (icmp slt X, 0) in r147777. Adding more patterns should be straightforward now.

- Ben

> The bad thing about the current approach is, that the code only ever tries to combine icmps, which are direct parameters to the same "and"
> (or "or") instruction. If we have something like (icmp A & icmp B) & icmp C,
> A and C will never be combined, unless A and B could be combined first.