[llvm-commits] CVS: llvm/lib/Transforms/Scalar/InstructionCombining.cpp
Chris Lattner
clattner at apple.com
Tue Dec 12 22:42:26 PST 2006
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
More information about the llvm-commits
mailing list