[llvm] r185257 - InstCombine: Be more agressive optimizing 'udiv' instrs with 'select' denoms
Eli Friedman
eli.friedman at gmail.com
Sat Jun 29 09:15:32 PDT 2013
On Sat, Jun 29, 2013 at 1:40 AM, David Majnemer <david.majnemer at gmail.com>wrote:
> Author: majnemer
> Date: Sat Jun 29 03:40:07 2013
> New Revision: 185257
>
> URL: http://llvm.org/viewvc/llvm-project?rev=185257&view=rev
> Log:
> InstCombine: Be more agressive optimizing 'udiv' instrs with 'select'
> denoms
>
> Real world code sometimes has the denominator of a 'udiv' be a
> 'select'. LLVM can handle such cases but only when the 'select'
> operands are symmetric in structure (both select operands are a constant
> power of two or a left shift, etc.). This falls apart if we are dealt a
> 'udiv' where the code is not symetric or if the select operands lead us
> to more select instructions.
>
> Instead, we should treat the LHS and each select operand as a distinct
> divide operation and try to optimize them independently. If we can
> to simplify each operation, then we can replace the 'udiv' with, say, a
> 'lshr' that has a new select with a bunch of new operands for the
> select.
>
> Modified:
> llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
> llvm/trunk/test/Transforms/InstCombine/div-shift.ll
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp?rev=185257&r1=185256&r2=185257&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
> (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp Sat Jun
> 29 03:40:07 2013
> @@ -705,26 +705,27 @@ static Value *dyn_castZExtVal(Value *V,
> return 0;
> }
>
> -Instruction *InstCombiner::visitUDiv(BinaryOperator &I) {
> - Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);
> -
> - if (Value *V = SimplifyUDivInst(Op0, Op1, TD))
> - return ReplaceInstUsesWith(I, V);
> -
> - // Handle the integer div common cases
> - if (Instruction *Common = commonIDivTransforms(I))
> - return Common;
> +const unsigned MaxDepth = 6;
>
> +// \brief Recursively visits the possible right hand operands of a udiv
> +// instruction, seeing through select instructions, to determine if we can
> +// replace the udiv with something simpler. If we find that an operand
> is not
> +// able to simplify the udiv, we abort the entire transformation.
> +//
> +// Inserts any intermediate instructions used for the simplification into
> +// NewInstrs and returns a new instruction that depends upon them.
> +static Instruction *visitUDivOperand(Value *Op0, Value *Op1,
> + const BinaryOperator &I,
> + SmallVectorImpl<Instruction *>
> &NewInstrs,
> + unsigned Depth = 0) {
> {
> // X udiv 2^C -> X >> C
> // Check to see if this is an unsigned division with an exact power
> of 2,
> // if so, convert to a right shift.
> const APInt *C;
> if (match(Op1, m_Power2(C))) {
> - BinaryOperator *LShr =
> - BinaryOperator::CreateLShr(Op0,
> - ConstantInt::get(Op0->getType(),
> - C->logBase2()));
> + BinaryOperator *LShr = BinaryOperator::CreateLShr(
> + Op0, ConstantInt::get(Op0->getType(), C->logBase2()));
> if (I.isExact()) LShr->setIsExact();
> return LShr;
> }
> @@ -733,51 +734,68 @@ Instruction *InstCombiner::visitUDiv(Bin
> if (ConstantInt *C = dyn_cast<ConstantInt>(Op1)) {
> // X udiv C, where C >= signbit
> if (C->getValue().isNegative()) {
> - Value *IC = Builder->CreateICmpULT(Op0, C);
> + ICmpInst *IC = new ICmpInst(ICmpInst::ICMP_ULT, Op0, C);
> + NewInstrs.push_back(IC);
> +
> return SelectInst::Create(IC, Constant::getNullValue(I.getType()),
> ConstantInt::get(I.getType(), 1));
> }
> }
>
> - // (x lshr C1) udiv C2 --> x udiv (C2 << C1)
> - if (ConstantInt *C2 = dyn_cast<ConstantInt>(Op1)) {
> - Value *X;
> - ConstantInt *C1;
> - if (match(Op0, m_LShr(m_Value(X), m_ConstantInt(C1)))) {
> - APInt NC =
> C2->getValue().shl(C1->getLimitedValue(C1->getBitWidth()-1));
> - return BinaryOperator::CreateUDiv(X, Builder->getInt(NC));
> - }
> - }
> -
> // X udiv (C1 << N), where C1 is "1<<C2" --> X >> (N+C2)
> { const APInt *CI; Value *N;
> if (match(Op1, m_Shl(m_Power2(CI), m_Value(N))) ||
> match(Op1, m_ZExt(m_Shl(m_Power2(CI), m_Value(N))))) {
> - if (*CI != 1)
> - N = Builder->CreateAdd(N,
> - ConstantInt::get(N->getType(),
> CI->logBase2()));
> - if (ZExtInst *Z = dyn_cast<ZExtInst>(Op1))
> - N = Builder->CreateZExt(N, Z->getDestTy());
> - if (I.isExact())
> - return BinaryOperator::CreateExactLShr(Op0, N);
> - return BinaryOperator::CreateLShr(Op0, N);
> + if (*CI != 1) {
> + N = BinaryOperator::CreateAdd(
> + N, ConstantInt::get(N->getType(), CI->logBase2()));
> + NewInstrs.push_back(cast<Instruction>(N));
> + }
> + if (ZExtInst *Z = dyn_cast<ZExtInst>(Op1)) {
> + N = new ZExtInst(N, Z->getDestTy());
> + NewInstrs.push_back(cast<Instruction>(N));
> + }
> + BinaryOperator *LShr = BinaryOperator::CreateLShr(Op0, N);
> + if (I.isExact()) LShr->setIsExact();
> + return LShr;
> }
> }
>
> - // udiv X, (Select Cond, C1, C2) --> Select Cond, (shr X, C1), (shr X,
> C2)
> - // where C1&C2 are powers of two.
> - { Value *Cond; const APInt *C1, *C2;
> - if (match(Op1, m_Select(m_Value(Cond), m_Power2(C1), m_Power2(C2)))) {
> - // Construct the "on true" case of the select
> - Value *TSI = Builder->CreateLShr(Op0, C1->logBase2(),
> Op1->getName()+".t",
> - I.isExact());
> -
> - // Construct the "on false" case of the select
> - Value *FSI = Builder->CreateLShr(Op0, C2->logBase2(),
> Op1->getName()+".f",
> - I.isExact());
> + // The remaining tests are all recursive, so bail out if we hit the
> limit.
> + if (Depth++ == MaxDepth)
> + return 0;
> +
> + if (SelectInst *SI = dyn_cast<SelectInst>(Op1))
> + if (Instruction *LHS =
> + visitUDivOperand(Op0, SI->getOperand(1), I, NewInstrs)) {
> + NewInstrs.push_back(LHS);
> + if (Instruction *RHS =
> + visitUDivOperand(Op0, SI->getOperand(2), I, NewInstrs)) {
> + NewInstrs.push_back(RHS);
> + return SelectInst::Create(SI->getCondition(), LHS, RHS);
> + }
> + }
> +
> + return 0;
> +}
> +
> +Instruction *InstCombiner::visitUDiv(BinaryOperator &I) {
> + Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);
> +
> + if (Value *V = SimplifyUDivInst(Op0, Op1, TD))
> + return ReplaceInstUsesWith(I, V);
> +
> + // Handle the integer div common cases
> + if (Instruction *Common = commonIDivTransforms(I))
> + return Common;
>
> - // construct the select instruction and return it.
> - return SelectInst::Create(Cond, TSI, FSI);
> + // (x lshr C1) udiv C2 --> x udiv (C2 << C1)
> + if (ConstantInt *C2 = dyn_cast<ConstantInt>(Op1)) {
> + Value *X;
> + ConstantInt *C1;
> + if (match(Op0, m_LShr(m_Value(X), m_ConstantInt(C1)))) {
> + APInt NC =
> C2->getValue().shl(C1->getLimitedValue(C1->getBitWidth()-1));
> + return BinaryOperator::CreateUDiv(X, Builder->getInt(NC));
> }
> }
>
> @@ -788,6 +806,21 @@ Instruction *InstCombiner::visitUDiv(Bin
> I.isExact()),
> I.getType());
>
> + // (LHS udiv (select (select (...)))) -> (LHS >> (select (select
> (...))))
> + SmallVector<Instruction *, 4> NewInstrs;
> + Instruction *RetI = visitUDivOperand(Op0, Op1, I, NewInstrs);
> + for (unsigned i = 0, e = NewInstrs.size(); i != e; i++)
> + // If we managed to replace the UDiv completely, insert the new
> intermediate
> + // instructions before where the UDiv was.
> + // If we couldn't, we must clean up after ourselves by deleting the
> new
> + // instructions.
> + if (RetI)
> + NewInstrs[i]->insertBefore(&I);
> + else
> + delete NewInstrs[i];
> + if (RetI)
> + return RetI;
>
>
"Create a bunch of instructions, then delete them" is a pattern we
generally try to avoid in InstCombine. I can see why you chose it here,
but please don't; it's a bad idea in terms of performance.
-Eli
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130629/bce303a2/attachment.html>
More information about the llvm-commits
mailing list