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. _______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
