[llvm-commits] [llvm] r53271 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/2008-07-08-SubAnd.ll

[Nick Lewycky]

Duncan Sands wrote:
> Hi Nick,
> 
>>>> +        // (A - N) & AndRHS -> -N & AndRHS where A & AndRHS == 0
>>> this seems to be wrong for A = 5, N = 1, AndRHS = 2.
>> Thanks Duncan. The fix I used was to replace 'where A & AndRHS' with 
>> 'where A & Mask' such that Mask = all 1's starting with the first 1 bit 
>> in AndRHS. That way AndRHS=2 causes Mask=3 and A&Mask = 1.
> 
> this looks ok: if M = Mask+1, then A & Mask = 0 means A mod M = 0.
> Modulo arithmetic gives: (A - N) mod M = -N mod M.  Doing an additional
> & AndRHS on the left- and right-hand side of this gives your simplification.

That's elegant. I certainly didn't work it through like that!

> However, since this seems to be a combination of two simplifications, namely
> modulo arithmetic and the transform (& Mask & AndRHS -> & AndRHS), mightn't
> it be better to teach instcombine about each simplification individually?
> In fact I'm kind of surprised it doesn't do these already.

I don't quite follow. How else, besides the presence of an 'and' 
instruction, would it know that's operating in a modulo space smaller 
than the one defined by the bitwidth of the integer type?

Were you suggesting that we look for 'urem' instructions where the right 
hand side isn't a power of 2? That wouldn't work correctly if the 
subtraction were to wrap around.

Nick