[llvm-commits] Div->[USF]Div Patch, Attempt #2

[Reid Spencer]

Chris,

A couple side notes on this one ...

On Wed, 2006-10-25 at 21:19 -0700, Chris Lattner wrote:

> 
> Overall, this patch looks very nice.  Please fix the one bug below
> (adding the regtest) and you are approved to check the whole mess in.

Thank you.


>     // (X / C1) / C2  -> X / (C1*C2)
>     if (Instruction *LHS = dyn_cast<Instruction>(Op0))
>       if (Instruction::BinaryOps(LHS->getOpcode()) == I.getOpcode())
> 
> 
> This code is a bit simpler as:
> 
> 
>     // (X / C1) / C2  -> X / (C1*C2)
>     if (BinaryOperator *LHS = dyn_cast<BinaryOperator>(Op0))
>       if (LHS->getOpcode() == I.getOpcode())
> 

Didn't apply this because it causes a compiler warning about
signed/unsigned comparison.

>   // udiv X, (Select Cond, C1, C2) --> Select Cond, (shr X, C1), (shr
> X, C2)
>   // where C1&C2 are powers of two.
> ...
>                 X = InsertNewInstBefore(
>                   new CastInst(X, X->getType()->getUnsignedVersion()),
> I);
> 
> 
> This (and similar cases) is easier/cleaner with InsertCastBefore.

Yes, I agree. I changed the ones in my code but there are actually
dozens (hundreds) of instances in the file (not related to my changes).
I'm going to commit the DIV patch first, then I'll come back and clean
up the others in a separate patch (tested, of course).

Thanks for your excellent reviews, Chris. I know it takes a lot of your
time and I appreciate that you so willingly give it. I'll apply what I
learned from DIV to the REM patch which I'm reviewing for Sheng soon.
Hopefully REM will fare better than DIV did. 

Reid.