From 453aa4fbf1083cc7f646a0ac21e2bcc384a91ae9 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Sun, 24 May 2009 18:06:31 +0000 Subject: [PATCH] Generalize SCEVExpander::visitAddRecExpr's GEP persuit, and avoid sending SCEVUnknowns to expandAddToGEP. This avoids the need for expandAddToGEP to bend the rules and peek into SCEVUnknown expressions. Factor out the code for testing whether a SCEV can be factored by a constant for use in a GEP index. This allows it to handle SCEVAddRecExprs, by recursing. As a result, SCEVExpander can now put more things in GEP indices, so it emits fewer explicit mul instructions. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@72366 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../llvm/Analysis/ScalarEvolutionExpander.h | 4 +- lib/Analysis/ScalarEvolutionExpander.cpp | 191 +++++++++++++----- test/Transforms/IndVarSimplify/addrec-gep.ll | 78 +++++++ .../IndVarSimplify/gep-with-mul-base.ll | 2 +- 4 files changed, 221 insertions(+), 54 deletions(-) create mode 100644 test/Transforms/IndVarSimplify/addrec-gep.ll diff --git a/include/llvm/Analysis/ScalarEvolutionExpander.h b/include/llvm/Analysis/ScalarEvolutionExpander.h index 8194555cdeb..7e0de47dc4f 100644 --- a/include/llvm/Analysis/ScalarEvolutionExpander.h +++ b/include/llvm/Analysis/ScalarEvolutionExpander.h @@ -110,8 +110,8 @@ namespace llvm { private: /// expandAddToGEP - Expand a SCEVAddExpr with a pointer type into a GEP /// instead of using ptrtoint+arithmetic+inttoptr. - Value *expandAddToGEP(const SCEVAddExpr *S, const PointerType *PTy, - const Type *Ty, Value *V); + Value *expandAddToGEP(const SCEVHandle *op_begin, const SCEVHandle *op_end, + const PointerType *PTy, const Type *Ty, Value *V); Value *expand(const SCEV *S); diff --git a/lib/Analysis/ScalarEvolutionExpander.cpp b/lib/Analysis/ScalarEvolutionExpander.cpp index fc66ddb6f48..7ebc00a19ae 100644 --- a/lib/Analysis/ScalarEvolutionExpander.cpp +++ b/lib/Analysis/ScalarEvolutionExpander.cpp @@ -144,17 +144,89 @@ Value *SCEVExpander::InsertBinop(Instruction::BinaryOps Opcode, Value *LHS, return BO; } +/// FactorOutConstant - Test if S is evenly divisible by Factor, using signed +/// division. If so, update S with Factor divided out and return true. +/// TODO: When ScalarEvolution gets a SCEVSDivExpr, this can be made +/// unnecessary; in its place, just signed-divide Ops[i] by the scale and +/// check to see if the divide was folded. +static bool FactorOutConstant(SCEVHandle &S, + const APInt &Factor, + ScalarEvolution &SE) { + // Everything is divisible by one. + if (Factor == 1) + return true; + + // For a Constant, check for a multiple of the given factor. + if (const SCEVConstant *C = dyn_cast(S)) + if (!C->getValue()->getValue().srem(Factor)) { + ConstantInt *CI = + ConstantInt::get(C->getValue()->getValue().sdiv(Factor)); + SCEVHandle Div = SE.getConstant(CI); + S = Div; + return true; + } + + // In a Mul, check if there is a constant operand which is a multiple + // of the given factor. + if (const SCEVMulExpr *M = dyn_cast(S)) + if (const SCEVConstant *C = dyn_cast(M->getOperand(0))) + if (!C->getValue()->getValue().srem(Factor)) { + std::vector NewMulOps(M->getOperands()); + NewMulOps[0] = + SE.getConstant(C->getValue()->getValue().sdiv(Factor)); + S = SE.getMulExpr(NewMulOps); + return true; + } + + // In an AddRec, check if both start and step are divisible. + if (const SCEVAddRecExpr *A = dyn_cast(S)) { + SCEVHandle Start = A->getStart(); + if (!FactorOutConstant(Start, Factor, SE)) + return false; + SCEVHandle Step = A->getStepRecurrence(SE); + if (!FactorOutConstant(Step, Factor, SE)) + return false; + S = SE.getAddRecExpr(Start, Step, A->getLoop()); + return true; + } + + return false; +} + /// expandAddToGEP - Expand a SCEVAddExpr with a pointer type into a GEP -/// instead of using ptrtoint+arithmetic+inttoptr. -Value *SCEVExpander::expandAddToGEP(const SCEVAddExpr *S, +/// instead of using ptrtoint+arithmetic+inttoptr. This helps +/// BasicAliasAnalysis analyze the result. However, it suffers from the +/// underlying bug described in PR2831. Addition in LLVM currently always +/// has two's complement wrapping guaranteed. However, the semantics for +/// getelementptr overflow are ambiguous. In the common case though, this +/// expansion gets used when a GEP in the original code has been converted +/// into integer arithmetic, in which case the resulting code will be no +/// more undefined than it was originally. +/// +/// Design note: It might seem desirable for this function to be more +/// loop-aware. If some of the indices are loop-invariant while others +/// aren't, it might seem desirable to emit multiple GEPs, keeping the +/// loop-invariant portions of the overall computation outside the loop. +/// However, there are a few reasons this is not done here. Hoisting simple +/// arithmetic is a low-level optimization that often isn't very +/// important until late in the optimization process. In fact, passes +/// like InstructionCombining will combine GEPs, even if it means +/// pushing loop-invariant computation down into loops, so even if the +/// GEPs were split here, the work would quickly be undone. The +/// LoopStrengthReduction pass, which is usually run quite late (and +/// after the last InstructionCombining pass), takes care of hoisting +/// loop-invariant portions of expressions, after considering what +/// can be folded using target addressing modes. +/// +Value *SCEVExpander::expandAddToGEP(const SCEVHandle *op_begin, + const SCEVHandle *op_end, const PointerType *PTy, const Type *Ty, Value *V) { const Type *ElTy = PTy->getElementType(); SmallVector GepIndices; - std::vector Ops = S->getOperands(); + std::vector Ops(op_begin, op_end); bool AnyNonZeroIndices = false; - Ops.pop_back(); // Decend down the pointer's type and attempt to convert the other // operands into GEP indices, at each level. The first index in a GEP @@ -167,45 +239,27 @@ Value *SCEVExpander::expandAddToGEP(const SCEVAddExpr *S, std::vector NewOps; std::vector ScaledOps; for (unsigned i = 0, e = Ops.size(); i != e; ++i) { + // Split AddRecs up into parts as either of the parts may be usable + // without the other. + if (const SCEVAddRecExpr *A = dyn_cast(Ops[i])) + if (!A->getStart()->isZero()) { + SCEVHandle Start = A->getStart(); + Ops.push_back(SE.getAddRecExpr(SE.getIntegerSCEV(0, A->getType()), + A->getStepRecurrence(SE), + A->getLoop())); + Ops[i] = Start; + ++e; + } + // If the scale size is not 0, attempt to factor out a scale. if (ElSize != 0) { - // For a Constant, check for a multiple of the pointer type's - // scale size. - if (const SCEVConstant *C = dyn_cast(Ops[i])) - if (!C->getValue()->getValue().srem(ElSize)) { - ConstantInt *CI = - ConstantInt::get(C->getValue()->getValue().sdiv(ElSize)); - SCEVHandle Div = SE.getConstant(CI); - ScaledOps.push_back(Div); - continue; - } - // In a Mul, check if there is a constant operand which is a multiple - // of the pointer type's scale size. - if (const SCEVMulExpr *M = dyn_cast(Ops[i])) - if (const SCEVConstant *C = dyn_cast(M->getOperand(0))) - if (!C->getValue()->getValue().srem(ElSize)) { - std::vector NewMulOps(M->getOperands()); - NewMulOps[0] = - SE.getConstant(C->getValue()->getValue().sdiv(ElSize)); - ScaledOps.push_back(SE.getMulExpr(NewMulOps)); - continue; - } - // In an Unknown, check if the underlying value is a Mul by a constant - // which is equal to the pointer type's scale size. - if (const SCEVUnknown *U = dyn_cast(Ops[i])) - if (BinaryOperator *BO = dyn_cast(U->getValue())) - if (BO->getOpcode() == Instruction::Mul) - if (ConstantInt *CI = dyn_cast(BO->getOperand(1))) - if (CI->getValue() == ElSize) { - ScaledOps.push_back(SE.getUnknown(BO->getOperand(0))); - continue; - } - // If the pointer type's scale size is 1, no scaling is necessary - // and any value can be used. - if (ElSize == 1) { - ScaledOps.push_back(Ops[i]); + SCEVHandle Op = Ops[i]; + if (FactorOutConstant(Op, ElSize, SE)) { + ScaledOps.push_back(Op); // Op now has ElSize factored out. continue; } } + // If the operand was not divisible, add it to the list of operands + // we'll scan next iteration. NewOps.push_back(Ops[i]); } Ops = NewOps; @@ -292,17 +346,14 @@ Value *SCEVExpander::visitAddExpr(const SCEVAddExpr *S) { const Type *Ty = SE.getEffectiveSCEVType(S->getType()); Value *V = expand(S->getOperand(S->getNumOperands()-1)); - // Turn things like ptrtoint+arithmetic+inttoptr into GEP. This helps - // BasicAliasAnalysis analyze the result. However, it suffers from the - // underlying bug described in PR2831. Addition in LLVM currently always - // has two's complement wrapping guaranteed. However, the semantics for - // getelementptr overflow are ambiguous. In the common case though, this - // expansion gets used when a GEP in the original code has been converted - // into integer arithmetic, in which case the resulting code will be no - // more undefined than it was originally. + // Turn things like ptrtoint+arithmetic+inttoptr into GEP. See the + // comments on expandAddToGEP for details. if (SE.TD) - if (const PointerType *PTy = dyn_cast(V->getType())) - return expandAddToGEP(S, PTy, Ty, V); + if (const PointerType *PTy = dyn_cast(V->getType())) { + const std::vector &Ops = S->getOperands(); + return expandAddToGEP(Ops.data(), Ops.data() + Ops.size() - 1, + PTy, Ty, V); + } V = InsertNoopCastOfTo(V, Ty); @@ -357,6 +408,27 @@ Value *SCEVExpander::visitUDivExpr(const SCEVUDivExpr *S) { return InsertBinop(Instruction::UDiv, LHS, RHS, InsertPt); } +/// Move parts of Base into Rest to leave Base with the minimal +/// expression that provides a pointer operand suitable for a +/// GEP expansion. +static void ExposePointerBase(SCEVHandle &Base, SCEVHandle &Rest, + ScalarEvolution &SE) { + while (const SCEVAddRecExpr *A = dyn_cast(Base)) { + Base = A->getStart(); + Rest = SE.getAddExpr(Rest, + SE.getAddRecExpr(SE.getIntegerSCEV(0, A->getType()), + A->getStepRecurrence(SE), + A->getLoop())); + } + if (const SCEVAddExpr *A = dyn_cast(Base)) { + Base = A->getOperand(A->getNumOperands()-1); + std::vector NewAddOps(A->op_begin(), A->op_end()); + NewAddOps.back() = Rest; + Rest = SE.getAddExpr(NewAddOps); + ExposePointerBase(Base, Rest, SE); + } +} + Value *SCEVExpander::visitAddRecExpr(const SCEVAddRecExpr *S) { const Type *Ty = SE.getEffectiveSCEVType(S->getType()); const Loop *L = S->getLoop(); @@ -365,8 +437,25 @@ Value *SCEVExpander::visitAddRecExpr(const SCEVAddRecExpr *S) { if (!S->getStart()->isZero()) { std::vector NewOps(S->getOperands()); NewOps[0] = SE.getIntegerSCEV(0, Ty); - Value *Rest = expand(SE.getAddRecExpr(NewOps, L)); - return expand(SE.getAddExpr(S->getStart(), SE.getUnknown(Rest))); + SCEVHandle Rest = SE.getAddRecExpr(NewOps, L); + + // Turn things like ptrtoint+arithmetic+inttoptr into GEP. See the + // comments on expandAddToGEP for details. + if (SE.TD) { + SCEVHandle Base = S->getStart(); + SCEVHandle RestArray[1] = Rest; + // Dig into the expression to find the pointer base for a GEP. + ExposePointerBase(Base, RestArray[0], SE); + // If we found a pointer, expand the AddRec with a GEP. + if (const PointerType *PTy = dyn_cast(Base->getType())) { + Value *StartV = expand(Base); + assert(StartV->getType() == PTy && "Pointer type mismatch for GEP!"); + return expandAddToGEP(RestArray, RestArray+1, PTy, Ty, StartV); + } + } + + Value *RestV = expand(Rest); + return expand(SE.getAddExpr(S->getStart(), SE.getUnknown(RestV))); } // {0,+,1} --> Insert a canonical induction variable into the loop! diff --git a/test/Transforms/IndVarSimplify/addrec-gep.ll b/test/Transforms/IndVarSimplify/addrec-gep.ll new file mode 100644 index 00000000000..132d4f8a871 --- /dev/null +++ b/test/Transforms/IndVarSimplify/addrec-gep.ll @@ -0,0 +1,78 @@ +; RUN: llvm-as < %s | opt -indvars | llvm-dis > %t +; RUN: grep getelementptr %t | count 1 +; RUN: grep {mul .*, 37} %t | count 1 +; RUN: grep {add .*, 5203} %t | count 1 +; RUN: not grep cast %t + +; This test tests several things. The load and store should use the +; same address instead of having it computed twice, and SCEVExpander should +; be able to reconstruct the full getelementptr, despite it having a few +; obstacles set in its way. + +target datalayout = "e-p:64:64:64" + +define void @foo(i64 %n, i64 %m, i64 %o, i64 %q, double* nocapture %p) nounwind { +entry: + %tmp = icmp sgt i64 %n, 0 ; [#uses=1] + br i1 %tmp, label %bb.nph3, label %return + +bb.nph: ; preds = %bb2.preheader + %tmp1 = mul i64 %tmp16, %i.02 ; [#uses=1] + %tmp2 = mul i64 %tmp19, %i.02 ; [#uses=1] + br label %bb1 + +bb1: ; preds = %bb2, %bb.nph + %j.01 = phi i64 [ %tmp9, %bb2 ], [ 0, %bb.nph ] ; [#uses=3] + %tmp3 = add i64 %j.01, %tmp1 ; [#uses=1] + %tmp4 = add i64 %j.01, %tmp2 ; [#uses=1] + %z0 = add i64 %tmp4, 5203 + %tmp5 = getelementptr double* %p, i64 %z0 ; [#uses=1] + %tmp6 = load double* %tmp5, align 8 ; [#uses=1] + %tmp7 = fdiv double %tmp6, 2.100000e+00 ; [#uses=1] + %z1 = add i64 %tmp4, 5203 + %tmp8 = getelementptr double* %p, i64 %z1 ; [#uses=1] + store double %tmp7, double* %tmp8, align 8 + %tmp9 = add i64 %j.01, 1 ; [#uses=2] + br label %bb2 + +bb2: ; preds = %bb1 + %tmp10 = icmp slt i64 %tmp9, %m ; [#uses=1] + br i1 %tmp10, label %bb1, label %bb2.bb3_crit_edge + +bb2.bb3_crit_edge: ; preds = %bb2 + br label %bb3 + +bb3: ; preds = %bb2.preheader, %bb2.bb3_crit_edge + %tmp11 = add i64 %i.02, 1 ; [#uses=2] + br label %bb4 + +bb4: ; preds = %bb3 + %tmp12 = icmp slt i64 %tmp11, %n ; [#uses=1] + br i1 %tmp12, label %bb2.preheader, label %bb4.return_crit_edge + +bb4.return_crit_edge: ; preds = %bb4 + br label %bb4.return_crit_edge.split + +bb4.return_crit_edge.split: ; preds = %bb.nph3, %bb4.return_crit_edge + br label %return + +bb.nph3: ; preds = %entry + %tmp13 = icmp sgt i64 %m, 0 ; [#uses=1] + %tmp14 = mul i64 %n, 37 ; [#uses=1] + %tmp15 = mul i64 %tmp14, %o ; [#uses=1] + %tmp16 = mul i64 %tmp15, %q ; [#uses=1] + %tmp17 = mul i64 %n, 37 ; [#uses=1] + %tmp18 = mul i64 %tmp17, %o ; [#uses=1] + %tmp19 = mul i64 %tmp18, %q ; [#uses=1] + br i1 %tmp13, label %bb.nph3.split, label %bb4.return_crit_edge.split + +bb.nph3.split: ; preds = %bb.nph3 + br label %bb2.preheader + +bb2.preheader: ; preds = %bb.nph3.split, %bb4 + %i.02 = phi i64 [ %tmp11, %bb4 ], [ 0, %bb.nph3.split ] ; [#uses=3] + br i1 true, label %bb.nph, label %bb3 + +return: ; preds = %bb4.return_crit_edge.split, %entry + ret void +} diff --git a/test/Transforms/IndVarSimplify/gep-with-mul-base.ll b/test/Transforms/IndVarSimplify/gep-with-mul-base.ll index 0e5e1067177..e63c88c65e6 100644 --- a/test/Transforms/IndVarSimplify/gep-with-mul-base.ll +++ b/test/Transforms/IndVarSimplify/gep-with-mul-base.ll @@ -1,6 +1,6 @@ ; RUN: llvm-as < %s | opt -indvars | llvm-dis > %t ; RUN: grep add %t | count 8 -; RUN: grep mul %t | count 9 +; RUN: grep mul %t | count 7 define void @foo(i64 %n, i64 %m, i64 %o, double* nocapture %p) nounwind { entry: