This code (around line 2173): CastInst *BoolCast = 0; if (CastInst *CI = dyn_cast<CastInst>(I.getOperand(0))) if (CI->getOperand(0)->getType() == Type::BoolTy) BoolCast = CI; if (!BoolCast) if (CastInst *CI = dyn_cast<CastInst>(I.getOperand(1))) if (CI->getOperand(0)->getType() == Type::BoolTy) BoolCast = CI;
Should *only* allow zext from bool (not any cast). This is a serious bug from the original cast patch. > @@ -2190,7 +2193,7 @@ > SCOpTy- > >getPrimitiveSizeInBits()-1); > if (SCIOp0->getType()->isUnsigned()) { > const Type *NewTy = SCIOp0->getType()->getSignedVersion(); > - SCIOp0 = InsertCastBefore(SCIOp0, NewTy, I); > + SCIOp0 = InsertCastBefore(Instruction::BitCast, SCIOp0, > NewTy, I); > } This cast be removed now that ashr is signless. > @@ -2863,12 +2872,14 @@ > Constant *ShrMask = ConstantExpr::getLShr(AllOne, OpRHS); > Constant *CI = ConstantExpr::getAnd(AndRHS, ShrMask); > if (CI == AndRHS) { // Masking out bits shifted in. > + // (Val ashr C1) & C2 -> (Val lshr C1) & C2 > // Make the argument unsigned. > Value *ShVal = Op->getOperand(0); > ShVal = InsertNewInstBefore(new ShiftInst > (Instruction::LShr, ShVal, > OpRHS, Op- > >getName()), > TheAnd); > - Value *AndRHS2 = ConstantExpr::getCast(AndRHS, ShVal- > >getType()); > + Value *AndRHS2 = ConstantExpr::getBitCast(AndRHS, ShVal- > >getType()); > + This cast looks completely dead, if so, it should just be removed. > @@ -2897,9 +2908,9 @@ > InsertNewInstBefore(Add, IB); > // Convert to unsigned for the comparison. > const Type *UnsType = Add->getType()->getUnsignedVersion(); > - Value *OffsetVal = InsertCastBefore(Add, UnsType, IB); > + Value *OffsetVal = InsertCastBefore(Instruction::BitCast, Add, > UnsType, IB); > AddCST = ConstantExpr::getAdd(AddCST, Hi); > - AddCST = ConstantExpr::getCast(AddCST, UnsType); > + AddCST = ConstantExpr::getBitCast(AddCST, UnsType); > return new SetCondInst(Instruction::SetLT, OffsetVal, AddCST); If only we had signless comparisons! :-) > @@ -3917,11 +3930,11 @@ > for (unsigned i = 1, e = GEP->getNumOperands(); i != e; ++i, + > +GTI) { > Value *Op = GEP->getOperand(i); > uint64_t Size = TD.getTypeSize(GTI.getIndexedType()) & > PtrSizeMask; > - Constant *Scale = ConstantExpr::getCast(ConstantInt::get > (UIntPtrTy, Size), > + Constant *Scale = ConstantExpr::getBitCast(ConstantInt::get > (UIntPtrTy, Size), > SIntPtrTy); This doesn't fit in 80 columns. Further, you can just use: > Constant *Scale = ConstantInt::get(SIntPtrTy, Size); Now that Constant[SU]Int got merged into ConstantInt. This code: // Check to see if there is a noop-cast between the shift and the and. if (!Shift) { if (CastInst *CI = dyn_cast<CastInst>(LHSI->getOperand(0))) if (CI->getOperand(0)->getType()->isIntegral() && CI->getOperand(0)->getType()- >getPrimitiveSizeInBits() == CI->getType()->getPrimitiveSizeInBits()) Shift = dyn_cast<ShiftInst>(CI->getOperand(0)); } Should check for bitcast with integral src/dest instead of checking sizes etc. This code: // Make sure we insert a logical shift. Constant *NewAndCST = AndCST; if (AndCST->getType()->isSigned()) NewAndCST = ConstantExpr::getBitCast(AndCST, AndCST->getType()- >getUnsignedVersion()); NS = new ShiftInst(Instruction::LShr, NewAndCST, Shift->getOperand(1), "tmp"); You can drop the cast now that the shift is signless. The two casts here can be eliminated now that shifts are signless: // (X >>s C1) << C2 where C1 > C2 === (X >>s (C1-C2)) & mask Op = InsertCastBefore(Instruction::BitCast, Mask, I.getType()->getSignedVersion(), I); Instruction *Shift = new ShiftInst(ShiftOp->getOpcode(), Op, ConstantInt::get(Type::UByteTy, ShiftAmt1- ShiftAmt2)); InsertNewInstBefore(Shift, I); C = ConstantIntegral::getAllOnesValue(Shift->getType()); C = ConstantExpr::getShl(C, Op1); Mask = BinaryOperator::createAnd(Shift, C, Op->getName() +".mask"); InsertNewInstBefore(Mask, I); return CastInst::create(Instruction::BitCast, Mask, I.getType ()); > @@ -5874,10 +5893,15 @@ > if (DestBitSize == SrcBitSize || > !ValueRequiresCast(Op1, DestTy,TD) || > !ValueRequiresCast(Op0, DestTy, TD)) { > - Value *Op0c = InsertOperandCastBefore(Op0, DestTy, SrcI); > - Value *Op1c = InsertOperandCastBefore(Op1, DestTy, SrcI); > - return BinaryOperator::create(cast<BinaryOperator>(SrcI) > - ->getOpcode(), Op0c, Op1c); > + unsigned Op0BitSize = Op0->getType()- > >getPrimitiveSizeInBits(); > + Instruction::CastOps opcode = > + (Op0BitSize > DestBitSize ? Instruction::Trunc : > + (Op0BitSize == DestBitSize ? Instruction::BitCast : > + Op0->getType()->isSigned() ? > Instruction::SExt :Instruction::ZExt)); This is a serious bug. You can't look at the type of the operand to determine the type of the cast! Replace this with: Instruction::CastOps opcode = CI.getOpcode(); > @@ -6014,13 +6042,18 @@ > > // Get a mask for the bits shifting in. > uint64_t Mask = (~0ULL >> (64-ShAmt)) << DestBitWidth; > - if (SrcI->hasOneUse() && MaskedValueIsZero(SrcI->getOperand > (0), Mask)) { > + Value* SrcIOp0 = SrcI->getOperand(0); > + if (SrcI->hasOneUse() && MaskedValueIsZero(SrcIOp0, Mask)) { > if (ShAmt >= DestBitWidth) // All zeros. > return ReplaceInstUsesWith(CI, Constant::getNullValue > (Ty)); > > // Okay, we can shrink this. Truncate the input, then > return a new > // shift. > - Value *V = InsertCastBefore(SrcI->getOperand(0), Ty, CI); > + Instruction::CastOps opcode = > + (SrcIOp0->getType()->getPrimitiveSizeInBits() == > + Ty->getPrimitiveSizeInBits() ? Instruction::BitCast : > + Instruction::Trunc); This is always trunc. > @@ -7316,15 +7355,18 @@ > return 0; > } > > -static Value *InsertSignExtendToPtrTy(Value *V, const Type *DTy, > - Instruction *InsertPoint, > - InstCombiner *IC) { > - unsigned PS = IC->getTargetData().getPointerSize(); > - const Type *VTy = V->getType(); > - if (!VTy->isSigned() && VTy->getPrimitiveSize() < PS) > - // We must insert a cast to ensure we sign-extend. > - V = IC->InsertCastBefore(V, VTy->getSignedVersion(), > *InsertPoint); > - return IC->InsertCastBefore(V, DTy, *InsertPoint); > +static Value *InsertCastToIntPtrTy(Value *V, const Type *DTy, > + Instruction *InsertPoint, > + InstCombiner *IC) { Why did you rename this? The previous name was better (indicated sign extension). > + unsigned PtrSize = IC->getTargetData().getPointerSize(); This isn't right. Two callers pass in intptr_t, but two pass in something else. You should use DTy->getPrimitiveSize() instead of TD.getPointerSize() > + unsigned VTySize = V->getType()->getPrimitiveSize(); > + // We must cast correctly to the pointer type. Ensure that we > + // sign extend the integer value if it is smaller as this is > + // used for address computation. > + Instruction::CastOps opcode = > + (VTySize < PtrSize ? Instruction::SExt : > + (VTySize == PtrSize ? Instruction::BitCast : > Instruction::Trunc)); > + return IC->InsertCastBefore(opcode, V, DTy, *InsertPoint); > } > @@ -8677,8 +8721,8 @@ > std::vector<Value*> Ops(CE->op_begin()+1, CE->op_end()); > uint64_t Offset = TD->getIndexedOffset(Ptr->getType(), Ops); > Constant *C = ConstantInt::get(Type::ULongTy, Offset); > + C = ConstantExpr::getIntegerCast(C, TD->getIntPtrType(), > true /*SExt*/); Just use: > Constant *C = ConstantInt::get(TD->getIntPtrType(), Offset); -Chris _______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits