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

[Duncan Sands]

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.
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.

Ciao,

Duncan.