Page MenuHomePhabricator
Differential D124369 Diff 424850 llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp

Changeset View

Standalone View

View Options

llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp

Show First 20 Lines • Show All 416 Lines • ▼ Show 20 Lines if (Op1->hasOneUse() && match(Op1, m_SRem(m_Value(A), m_Constant(C))) &&
// FIXME: Should this get moved into SimplifyDemandedBits by saying we don't // FIXME: Should this get moved into SimplifyDemandedBits by saying we don't
// demand the sign bit (and many others) here?? // demand the sign bit (and many others) here??
Constant *Mask = ConstantExpr::getSub(C, ConstantInt::get(I.getType(), 1)); Constant *Mask = ConstantExpr::getSub(C, ConstantInt::get(I.getType(), 1));
Value *Rem = Builder.CreateAnd(A, Mask, Op1->getName()); Value *Rem = Builder.CreateAnd(A, Mask, Op1->getName());
return replaceOperand(I, 1, Rem); return replaceOperand(I, 1, Rem);
} }
if (Instruction *Logic = foldShiftOfShiftedLogic(I, Builder)) if (Instruction *Logic = foldShiftOfShiftedLogic(I, Builder))
return Logic; return Logic;
spatelUnsubmitted
Not Done
ReplyInline Actions

This is more verbose than needed - if we're using &I, then we can just grab the opcode from it via I.getOpcode().
So we can either pass things in as params or use refs, instead of partly using both ways?

auto isSuitableForPreShift = [](Instruction &I, const APInt *AC, unsigned PosOffset) {
auto isSuitableForPreShift = [&I, AC, PosOffset]() {
spatel: This is more verbose than needed - if we're using &I, then we can just grab the opcode from it…
return nullptr; return nullptr;
} }
/// Return true if we can simplify two logical (either left or right) shifts /// Return true if we can simplify two logical (either left or right) shifts
/// that have constant shift amounts: OuterShift (InnerShift X, C1), C2. /// that have constant shift amounts: OuterShift (InnerShift X, C1), C2.
static bool canEvaluateShiftedShift(unsigned OuterShAmt, bool IsOuterShl, static bool canEvaluateShiftedShift(unsigned OuterShAmt, bool IsOuterShl,
Instruction *InnerShift, Instruction *InnerShift,
InstCombinerImpl &IC, Instruction *CxtI) { InstCombinerImpl &IC, Instruction *CxtI) {
Show All 14 Linesstatic bool canEvaluateShiftedShift(unsigned OuterShAmt, bool IsOuterShl,
// Equal shift amounts in opposite directions become bitwise 'and': // Equal shift amounts in opposite directions become bitwise 'and':
// lshr (shl X, C), C --> and X, C' // lshr (shl X, C), C --> and X, C'
// shl (lshr X, C), C --> and X, C' // shl (lshr X, C), C --> and X, C'
if (*InnerShiftConst == OuterShAmt) if (*InnerShiftConst == OuterShAmt)
return true; return true;
// If the 2nd shift is bigger than the 1st, we can fold: // If the 2nd shift is bigger than the 1st, we can fold:
// lshr (shl X, C1), C2 --> and (shl X, C1 - C2), C3 // lshr (shl X, C1), C2 --> and (shl X, C1 - C2), C3
// shl (lshr X, C1), C2 --> and (lshr X, C1 - C2), C3 // shl (lshr X, C1), C2 --> and (lshr X, C1 - C2), C3
spatelUnsubmitted
Not Done
ReplyInline Actions

Instead of a lambda + switch for setting the flags, I'd prefer the shorter and more direct:

if (I.getOpcode() == Instruction::Shl) {
   NewShiftOp->setHasNoUnsignedWrap(I.hasNoUnsignedWrap());
else
   NewShiftOp->setIsExact();

That might tilt the whole thing towards a lambda-less implementation, but you can decide which is easier to read.

spatel: Instead of a lambda + switch for setting the flags, I'd prefer the shorter and more direct…
// but it isn't profitable unless we know the and'd out bits are already zero. // but it isn't profitable unless we know the and'd out bits are already zero.
// Also, check that the inner shift is valid (less than the type width) or // Also, check that the inner shift is valid (less than the type width) or
// we'll crash trying to produce the bit mask for the 'and'. // we'll crash trying to produce the bit mask for the 'and'.
unsigned TypeWidth = InnerShift->getType()->getScalarSizeInBits(); unsigned TypeWidth = InnerShift->getType()->getScalarSizeInBits();
if (InnerShiftConst->ugt(OuterShAmt) && InnerShiftConst->ult(TypeWidth)) { if (InnerShiftConst->ugt(OuterShAmt) && InnerShiftConst->ult(TypeWidth)) {
unsigned InnerShAmt = InnerShiftConst->getZExtValue(); unsigned InnerShAmt = InnerShiftConst->getZExtValue();
unsigned MaskShift = unsigned MaskShift =
IsInnerShl ? TypeWidth - InnerShAmt : InnerShAmt - OuterShAmt; IsInnerShl ? TypeWidth - InnerShAmt : InnerShAmt - OuterShAmt;
▲ Show 20 Lines • Show All 768 Lines • ▼ Show 20 LinesInstruction *InstCombinerImpl::visitLShr(BinaryOperator &I) {
// Transform (x << y) >> y to x & (-1 >> y) // Transform (x << y) >> y to x & (-1 >> y)
Value *X; Value *X;
if (match(Op0, m_OneUse(m_Shl(m_Value(X), m_Specific(Op1))))) { if (match(Op0, m_OneUse(m_Shl(m_Value(X), m_Specific(Op1))))) {
Constant *AllOnes = ConstantInt::getAllOnesValue(Ty); Constant *AllOnes = ConstantInt::getAllOnesValue(Ty);
Value *Mask = Builder.CreateLShr(AllOnes, Op1); Value *Mask = Builder.CreateLShr(AllOnes, Op1);
return BinaryOperator::CreateAnd(Mask, X); return BinaryOperator::CreateAnd(Mask, X);
} }
// Try to pre-shift a constant shifted by a variable amount:
// C >> (X - AddC) --> (C << AddC) >> X
// This requires a negative offset constant.
const APInt *AddC;
unsigned BitWidth = Ty->getScalarSizeInBits();
if (match(Op0, m_APInt(C)) && match(Op1, m_Add(m_Value(X), m_APInt(AddC))) &&
AddC->isNegative() && (-*AddC).ult(BitWidth)) {
assert(!C->isZero() && "Expected simplify of shifted zero");
unsigned PosOffset = (-*AddC).getZExtValue();
if (C->eq(C->shl(PosOffset).lshr(PosOffset))) {
Constant *NewC = ConstantInt::get(Ty, C->shl(PosOffset));
Instruction *NewLShr = BinaryOperator::CreateLShr(NewC, X);
return NewLShr;
}
}
return nullptr; return nullptr;
} }
Instruction * Instruction *
InstCombinerImpl::foldVariableSignZeroExtensionOfVariableHighBitExtract( InstCombinerImpl::foldVariableSignZeroExtensionOfVariableHighBitExtract(
BinaryOperator &OldAShr) { BinaryOperator &OldAShr) {
assert(OldAShr.getOpcode() == Instruction::AShr && assert(OldAShr.getOpcode() == Instruction::AShr &&
"Must be called with arithmetic right-shift instruction only."); "Must be called with arithmetic right-shift instruction only.");
▲ Show 20 Lines • Show All 172 Lines • ▼ Show 20 LinesInstruction *InstCombinerImpl::visitAShr(BinaryOperator &I) {
// ashr (xor %x, -1), %y --> xor (ashr %x, %y), -1 // ashr (xor %x, -1), %y --> xor (ashr %x, %y), -1
if (match(Op0, m_OneUse(m_Not(m_Value(X))))) { if (match(Op0, m_OneUse(m_Not(m_Value(X))))) {
// Note that we must drop 'exact'-ness of the shift! // Note that we must drop 'exact'-ness of the shift!
// Note that we can't keep undef's in -1 vector constant! // Note that we can't keep undef's in -1 vector constant!
auto *NewAShr = Builder.CreateAShr(X, Op1, Op0->getName() + ".not"); auto *NewAShr = Builder.CreateAShr(X, Op1, Op0->getName() + ".not");
return BinaryOperator::CreateNot(NewAShr); return BinaryOperator::CreateNot(NewAShr);
} }
// Try to pre-shift a constant shifted by a variable amount:
// C >> (X - AddC) --> (C << AddC) >> X
// This requires the exact flag and a negative offset constant.
const APInt *AddC;
const APInt *C;
if (I.isExact() && match(Op0, m_APInt(C)) &&
match(Op1, m_Add(m_Value(X), m_APInt(AddC))) && AddC->isNegative() &&
(-*AddC).ult(BitWidth)) {
assert(!C->isZero() && "Expected simplify of shifted zero");
unsigned PosOffset = (-*AddC).getZExtValue();
if (C->eq(C->shl(PosOffset).ashr(PosOffset))) {
Constant *NewC = ConstantInt::get(Ty, C->shl(PosOffset));
Instruction *NewAShr = BinaryOperator::CreateAShr(NewC, X);
return NewAShr;
}
}
return nullptr; return nullptr;
} }