>
> Add an APInt version of SimplifyDemandedBits.
>
> Patch by Zhou Sheng.
Commenting on the version from mainline, comments preceeded with ***'s:
static void ComputeMaskedBits(Value *V, APInt Mask, APInt& KnownZero,
*** Mask should be passed by const&.
bool InstCombiner::SimplifyDemandedBits(Value *V, APInt DemandedMask,
*** Plz pass APInt's by const&, not by-value.
APInt& KnownZero, APInt&
KnownOne,
unsigned Depth) {
...
Instruction *I = dyn_cast<Instruction>(V);
if (!I) return false; // Only analyze instructions.
DemandedMask &= APInt::getAllOnesValue(BitWidth);
*** This &= is dead.
APInt LHSKnownZero(BitWidth, 0), LHSKnownOne(BitWidth, 0);
APInt &RHSKnownZero = KnownZero, &RHSKnownOne = KnownOne;
*** What is the point of the RHSKnownZero/RHSKnownOne references, plz
drop them.
switch (I->getOpcode()) {
...
case Instruction::Trunc: {
uint32_t truncBf =
cast<IntegerType>(I->getOperand(0)->getType())->getBitWidth();
if (SimplifyDemandedBits(I->getOperand(0), DemandedMask.zext
(truncBf),
RHSKnownZero.zext(truncBf), RHSKnownOne.zext(truncBf), Depth
+1))
*** Plz move the calls to zext out of line.
return true;
DemandedMask.trunc(BitWidth);
RHSKnownZero.trunc(BitWidth);
RHSKnownOne.trunc(BitWidth);
assert((RHSKnownZero & RHSKnownOne) == 0 &&
"Bits known to be one AND zero?");
break;
}
case Instruction::ZExt: {
// Compute the bits in the result that are not present in the
input.
const IntegerType *SrcTy = cast<IntegerType>(I->getOperand(0)-
>getType());
APInt NewBits(APInt::getAllOnesValue(BitWidth).shl(SrcTy-
>getBitWidth()));
*** This idiom occurs in many places, it should be a new
APInt::getHighBitsSet(width,numbits) method.
DemandedMask &= SrcTy->getMask().zext(BitWidth);
*** This should truncate DemandedMask then 'and' it.
uint32_t zextBf = SrcTy->getBitWidth();
if (SimplifyDemandedBits(I->getOperand(0), DemandedMask.trunc
(zextBf),
RHSKnownZero.trunc(zextBf), RHSKnownOne.trunc(zextBf),
Depth+1))
*** Truncs out of line plz.
return true;
}
case Instruction::SExt: {
// Compute the bits in the result that are not present in the
input.
const IntegerType *SrcTy = cast<IntegerType>(I->getOperand(0)-
>getType());
APInt NewBits(APInt::getAllOnesValue(BitWidth).shl(SrcTy-
>getBitWidth()));
*** Should use APInt::getHighBitsSet.
// Get the sign bit for the source type
APInt InSignBit(APInt::getSignBit(SrcTy->getPrimitiveSizeInBits
()));
InSignBit.zext(BitWidth);
APInt InputDemandedBits = DemandedMask &
SrcTy->getMask().zext(BitWidth);
*** The getMask().zext stuff should get 'APInt::getLowBitsSet(...)'.
..
uint32_t sextBf = SrcTy->getBitWidth();
if (SimplifyDemandedBits(I->getOperand(0),
InputDemandedBits.trunc(sextBf),
RHSKnownZero.trunc(sextBf), RHSKnownOne.trunc(sextBf),
Depth+1))
*** Plz don't nest the trunc calls.
return true;
case Instruction::Add: {
...
// If the top bit of the output is demanded, demand everything
from the
// input. Otherwise, we demand all the input bits except NLZ
top bits.
APInt InDemandedBits(APInt::getAllOnesValue(BitWidth).lshr(NLZ));
*** Should be APInt::getLowBitsSet.
...
bool CarryIn = false;
APInt CarryBits(BitWidth, 0);
const uint64_t *LHSKnownZeroRawVal = LHSKnownZero.getRawData(),
*RHSRawVal = RHSVal.getRawData();
for (uint32_t i = 0; i != RHSVal.getNumWords(); ++i) {
uint64_t AddVal = ~LHSKnownZeroRawVal[i] + RHSRawVal[i],
XorVal = ~LHSKnownZeroRawVal[i] ^ RHSRawVal[i];
uint64_t WordCarryBits = AddVal ^ XorVal + CarryIn;
if (AddVal < RHSRawVal[i])
CarryIn = true;
else
CarryIn = false;
CarryBits.setWordToValue(i, WordCarryBits);
}
*** Why aren't you doing this in terms of APInt operations?
...
// If the high-bits of this ADD are not demanded, then it does
not demand
// the high bits of its LHS or RHS.
if ((DemandedMask & APInt::getSignBit(BitWidth)) == 0) {
*** This seems like it should be: "if (DemandedMask[BitWidth-1] == 0)
{". Likewise in other places, grep for getSignBit to find them.
// Right fill the mask of bits for this ADD to demand the most
// significant bit and all those below it.
APInt DemandedFromOps = APInt::getAllOnesValue(BitWidth).lshr
(NLZ);
*** Use APInt::getLowBitsSet.
...
case Instruction::Sub:
// If the high-bits of this SUB are not demanded, then it does
not demand
// the high bits of its LHS or RHS.
if ((DemandedMask & APInt::getSignBit(BitWidth)) == 0) {
*** Use operator[]
// Right fill the mask of bits for this SUB to demand the most
// significant bit and all those below it.
unsigned NLZ = DemandedMask.countLeadingZeros();
APInt DemandedFromOps(APInt::getAllOnesValue(BitWidth).lshr
(NLZ));
*** Use getLowBitsSet.
case Instruction::Shl:
if (ConstantInt *SA = dyn_cast<ConstantInt>(I->getOperand(1))) {
uint64_t ShiftAmt = SA->getZExtValue();
if (SimplifyDemandedBits(I->getOperand(0), DemandedMask.lshr
(ShiftAmt),
*** Don't nest the lshr call, it makes it VERY easy to miss.
RHSKnownZero, RHSKnownOne, Depth+1))
return true;
...
// low bits known zero.
if (ShiftAmt)
RHSKnownZero |= APInt::getAllOnesValue(ShiftAmt).zextOrCopy
(BitWidth);
*** getLowBitsSet. If this is the only use of zextOrCopy, remove it.
case Instruction::LShr:
// For a logical shift right
if (ConstantInt *SA = dyn_cast<ConstantInt>(I->getOperand(1))) {
unsigned ShiftAmt = SA->getZExtValue();
APInt TypeMask(APInt::getAllOnesValue(BitWidth));
// Unsigned shift right.
if (SimplifyDemandedBits(I->getOperand(0),
(DemandedMask.shl(ShiftAmt)) & TypeMask,
*** The & is dead, move the shl out of line plz.
RHSKnownZero, RHSKnownOne, Depth+1))
return true;
assert((RHSKnownZero & RHSKnownOne) == 0 &&
"Bits known to be one AND zero?");
RHSKnownZero &= TypeMask;
RHSKnownOne &= TypeMask;
*** These &='s are dead.
RHSKnownZero = APIntOps::lshr(RHSKnownZero, ShiftAmt);
RHSKnownOne = APIntOps::lshr(RHSKnownOne, ShiftAmt);
if (ShiftAmt) {
// Compute the new bits that are at the top now.
APInt HighBits(APInt::getAllOnesValue(BitWidth).shl(
BitWidth - ShiftAmt));
*** Should use APInt::getHighBitsSet.
...
case Instruction::AShr:
*** Many of the same comments about LShr apply here.
...
// Handle the sign bits.
APInt SignBit(APInt::getSignBit(BitWidth));
// Adjust to where it is now in the mask.
SignBit = APIntOps::lshr(SignBit, ShiftAmt);
// If the input sign bit is known to be zero, or if none of
the top bits
// are demanded, turn this into an unsigned shift right.
if ((RHSKnownZero & SignBit) != 0 ||
*** All this logic playing with Sign bit should be eliminated and
replace with use of RHSKnownZero[] and RHSKnownOne[] (down below).
static void ComputeUnsignedMinMaxValuesFromKnownBits(const Type *Ty,
const APInt&
KnownZero,
const APInt&
KnownOne,
APInt& Min,
APInt& Max) {
uint32_t BitWidth = cast<IntegerType>(Ty)->getBitWidth();
assert(KnownZero.getBitWidth() == BitWidth &&
KnownOne.getBitWidth() == BitWidth &&
Min.getBitWidth() == BitWidth && Max.getBitWidth() &&
"Ty, KnownZero, KnownOne and Min, Max must have equal
bitwidth.");
APInt TypeBits(APInt::getAllOnesValue(BitWidth));
APInt UnknownBits = ~(KnownZero|KnownOne) & TypeBits;
*** TypeBits and the & are dead.
// The minimum value is when the unknown bits are all zeros.
Min = KnownOne;
// The maximum value is when the unknown bits are all ones.
Max = KnownOne|UnknownBits;
}
static void ComputeSignedMinMaxValuesFromKnownBits(const Type *Ty,
const APInt&
KnownZero,
const APInt&
KnownOne,
APInt& Min,
APInt& Max) {
uint32_t BitWidth = cast<IntegerType>(Ty)->getBitWidth();
assert(KnownZero.getBitWidth() == BitWidth &&
KnownOne.getBitWidth() == BitWidth &&
Min.getBitWidth() == BitWidth && Max.getBitWidth() ==
BitWidth &&
"Ty, KnownZero, KnownOne and Min, Max must have equal
bitwidth.");
APInt TypeBits(APInt::getAllOnesValue(BitWidth));
APInt UnknownBits = ~(KnownZero|KnownOne) & TypeBits;
*** TypeBits and the & are dead.
APInt SignBit(APInt::getSignBit(BitWidth));
*** Eliminate SignBit (and its uses) in favor of using UnknownBIts
[BitWidth-1] and Min[BitWidth-1] = 1, etc.
// The minimum value is when all unknown bits are zeros, EXCEPT
for the sign
// bit if it is unknown.
Min = KnownOne;
Max = KnownOne|UnknownBits;
if ((SignBit & UnknownBits) != 0) { // Sign bit is unknown
Min |= SignBit;
Max &= ~SignBit;
}
}
static bool ShrinkDemandedConstant(Instruction *I, unsigned OpNo,
APInt Demanded) {
*** Pass Demanded by const&.
assert(I && "No instruction?");
assert(OpNo < I->getNumOperands() && "Operand index too large");
// If the operand is not a constant integer, nothing to do.
ConstantInt *OpC = dyn_cast<ConstantInt>(I->getOperand(OpNo));
if (!OpC) return false;
// If there are no bits set that aren't demanded, nothing to do.
Demanded.zextOrTrunc(OpC->getValue().getBitWidth());
*** This zextOrTrunc is dead.
if ((~Demanded & OpC->getValue()) == 0)
return false;
// This instruction is producing bits that are not demanded.
Shrink the RHS.
Demanded &= OpC->getValue();
I->setOperand(OpNo, ConstantInt::get(Demanded));
return true;
}
Overall, nice job,
-Chris
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
