[llvm-commits] CVS: llvm/lib/Transforms/Scalar/InstructionCombining.cpp

[Reid Spencer]

Sheng, 

A correction to my last email ..

On Tue, 2007-03-27 at 22:13 -0700, Reid Spencer wrote:
> >      // If the sign bit of the input is known set or clear, then we know the
> >      // top bits of the result.
> > -    APInt InSignBit(APInt::getSignBit(SrcTy->getBitWidth()));
> > -    InSignBit.zext(BitWidth);
> > -    if ((KnownZero & InSignBit) != 0) {          // Input sign bit known zero
> > +    APInt NewBits(APInt::getHighBitsSet(BitWidth, BitWidth - SrcBitWidth));
> > +    if (KnownZero[SrcBitWidth-1])             // Input sign bit known zero
> 
> This doesn't look correct nor efficient to me. It should be testing for
> only the sign bit. YOu need an & not an |.  You're trying to determine
> if the sign bit of the SrcTy is set in KnownZero, right? So, why not:
> 
>    if (KnownZero.get(SrcBitWidth-1))
> 
> ?

My eyes are tired tonight. I didn't see the [...] and thought the [ was
| so please disregard. This looks okay.

> 
> >        KnownZero |= NewBits;
> > -      KnownOne &= ~NewBits;
> > -    } else if ((KnownOne & InSignBit) != 0) {    // Input sign bit known set
> > +    else if (KnownOne[SrcBitWidth-1])           // Input sign bit known set
> 
> Same issue as above.
> 
> >        KnownOne |= NewBits;
> > -      KnownZero &= ~NewBits;
> > -    } else {                              // Input sign bit unknown
> > -      KnownZero &= ~NewBits;
> > -      KnownOne &= ~NewBits;
> > -    }
> 
> Why did you delete this? What if the sign bit is unknown? (neither known
> one nor known zero).  Please revert.

This however is still a problem. It is quite possible for KnownZero[X]
and KnownOne[X] to both be false.

Reid.
> 
> >      return;
> >    }
> >    case Instruction::Shl:
> > @@ -760,7 +759,6 @@
> >      if (ConstantInt *SA = dyn_cast<ConstantInt>(I->getOperand(1))) {
> >        // Compute the new bits that are at the top now.
> >        uint64_t ShiftAmt = SA->getZExtValue();
> > -      APInt HighBits(APInt::getHighBitsSet(BitWidth, ShiftAmt));
> >        
> >        // Unsigned shift right.
> >        APInt Mask2(Mask.shl(ShiftAmt));
> > @@ -768,16 +766,16 @@
> >        assert((KnownZero & KnownOne) == 0&&"Bits known to be one AND zero?"); 
> >        KnownZero = APIntOps::lshr(KnownZero, ShiftAmt);
> >        KnownOne  = APIntOps::lshr(KnownOne, ShiftAmt);
> > -      KnownZero |= HighBits;  // high bits known zero.
> > +      // high bits known zero.
> > +      KnownZero |= APInt::getHighBitsSet(BitWidth, ShiftAmt);
> >        return;
> >      }
> >      break;
> >    case Instruction::AShr:
> > -    // (ushr X, C1) & C2 == 0   iff  (-1 >> C1) & C2 == 0
> > +    // (ashr X, C1) & C2 == 0   iff  (-1 >> C1) & C2 == 0
> >      if (ConstantInt *SA = dyn_cast<ConstantInt>(I->getOperand(1))) {
> >        // Compute the new bits that are at the top now.
> >        uint64_t ShiftAmt = SA->getZExtValue();
> > -      APInt HighBits(APInt::getHighBitsSet(BitWidth, ShiftAmt));
> >        
> >        // Signed shift right.
> >        APInt Mask2(Mask.shl(ShiftAmt));
> > @@ -789,11 +787,11 @@
> >        // Handle the sign bits and adjust to where it is now in the mask.
> >        APInt SignBit(APInt::getSignBit(BitWidth).lshr(ShiftAmt));
> >          
> > -      if ((KnownZero & SignBit) != 0) {       // New bits are known zero.
> > +      APInt HighBits(APInt::getHighBitsSet(BitWidth, ShiftAmt));
> > +      if (KnownZero[BitWidth-ShiftAmt-1])    // New bits are known zero.
> >          KnownZero |= HighBits;
> > -      } else if ((KnownOne & SignBit) != 0) { // New bits are known one.
> > +      else if (KnownOne[BitWidth-ShiftAmt-1])  // New bits are known one.
> >          KnownOne |= HighBits;
> > -      }
> >        return;
> >      }
> >      break;
> > 
> > 
> > 
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits