On Mar 5, 2007, at 1:39 PM, Reid Spencer wrote:
> Chris,
>
> While reviewing some of Sheng's patches, I think I found an
> existing bug
> in InstCombine. Please review this and if its okay I'll commit the
> patch:
>
> Index: InstructionCombining.cpp
> ===================================================================
> RCS
> file: /var/cvs/llvm/llvm/lib/Transforms/Scalar/
> InstructionCombining.cpp,v
> retrieving revision 1.647
> diff -t -d -u -p -5 -r1.647 InstructionCombining.cpp
> --- InstructionCombining.cpp 5 Mar 2007 00:11:19 -0000 1.647
> +++ InstructionCombining.cpp 5 Mar 2007 21:37:17 -0000
> @@ -2408,11 +2408,11 @@ Instruction *InstCombiner::visitUDiv(Bin
> // udiv X, (Select Cond, C1, C2) --> Select Cond, (shr X, C1), (shr
> X, C2)
> // where C1&C2 are powers of two.
> if (SelectInst *SI = dyn_cast<SelectInst>(Op1)) {
> if (ConstantInt *STO = dyn_cast<ConstantInt>(SI->getOperand(1)))
> if (ConstantInt *SFO = dyn_cast<ConstantInt>(SI->getOperand
> (2)))
> - if (!STO->isNullValue() && !STO->isNullValue()) {
> + if (!STO->isZero() && !SFO->isZero()) {
>
> While I've changed it to use the less expensive isZero, the real
> bug is
> that this is testing X & X. That is, the second term should check SF0
> against zero, not ST0. I can't think of any reason why testing ST0
> twice
> would be correct.
Interesting case. the full code looks like this:
if (!STO->isNullValue() && !STO->isNullValue()) {
uint64_t TVA = STO->getZExtValue(), FVA = SFO->getZExtValue
();
if (isPowerOf2_64(TVA) && isPowerOf2_64(FVA)) {
The isPowerOf2_64 calls check that the argument is not zero. As
such, you can drop the isNullValue checks entirely.
Thanks Reid,
-Chris
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
