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