<div dir="ltr">Argh, will revert.<div>Thanks!</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 14, 2016 at 3:23 PM, Evgenii Stepanov <span dir="ltr"><<a href="mailto:eugeni.stepanov@gmail.com" target="_blank">eugeni.stepanov@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This broke stuff.<br>
<a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/21045" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/21045</a><br>
<br>
ninja clang && bin/clang++ -D_DEBUG<br>
-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -Dcxx_EXPORTS<br>
-I/code/llvm/projects/libcxx/include -m64 -O3 -DNDEBUG -fPIC<br>
-UNDEBUG -std=c++11 -nostdinc++ -Wall -W -Wwrite-strings<br>
-Wno-unused-parameter -Wno-long-long -Werror=return-type -Wno-error<br>
-fPIC -c /code/llvm/projects/libcxx/src/locale.cpp<br>
<br>
PHINode should have one entry for each predecessor of its parent basic block!<br>
%incdec.ptr44326.lcssa = phi i32* [ %ind.escape, %middle.block380 ],<br>
[ %incdec.ptr44326, %for.cond41.for.end47_crit_edge.loopexit ], [<br>
%ind.end393, %middle.block380 ]<br>
fatal error: error in backend: Broken function found, compilation aborted!<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
On Tue, Jun 14, 2016 at 2:27 PM, Michael Kuperstein via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> Author: mkuper<br>
> Date: Tue Jun 14 16:27:27 2016<br>
> New Revision: 272715<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=272715&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=272715&view=rev</a><br>
> Log:<br>
> [LV] Enable vectorization of loops where the IV has an external use<br>
><br>
> Vectorizing loops with "escaping" IVs has been disabled since r190790, due to<br>
> PR17179. This re-enables it, with support for external use of both<br>
> "post-increment" (last iteration) and "pre-increment" (second-to-last iteration)<br>
> IVs.<br>
><br>
> Differential Revision: <a href="http://reviews.llvm.org/D21048" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21048</a><br>
><br>
> Added:<br>
> llvm/trunk/test/Transforms/LoopVectorize/iv_outside_user.ll<br>
> Modified:<br>
> llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp<br>
> llvm/trunk/test/Transforms/LoopVectorize/no_outside_user.ll<br>
><br>
> Modified: llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp?rev=272715&r1=272714&r2=272715&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp?rev=272715&r1=272714&r2=272715&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp Tue Jun 14 16:27:27 2016<br>
> @@ -355,6 +355,12 @@ protected:<br>
><br>
> /// Create an empty loop, based on the loop ranges of the old loop.<br>
> void createEmptyLoop();<br>
> +<br>
> + /// Set up the values of the IVs correctly when exiting the vector loop.<br>
> + void fixupIVUsers(PHINode *OrigPhi, const InductionDescriptor &II,<br>
> + Value *CountRoundDown, Value *EndValue,<br>
> + BasicBlock *MiddleBlock);<br>
> +<br>
> /// Create a new induction variable inside L.<br>
> PHINode *createInductionVariable(Loop *L, Value *Start, Value *End,<br>
> Value *Step, Instruction *DL);<br>
> @@ -1433,13 +1439,11 @@ private:<br>
> /// invariant.<br>
> void collectStridedAccess(Value *LoadOrStoreInst);<br>
><br>
> - /// \brief Returns true if we can vectorize using this PHI node as an<br>
> - /// induction.<br>
> - ///<br>
> /// Updates the vectorization state by adding \p Phi to the inductions list.<br>
> /// This can set \p Phi as the main induction of the loop if \p Phi is a<br>
> /// better choice for the main induction than the existing one.<br>
> - bool addInductionPhi(PHINode *Phi, InductionDescriptor ID);<br>
> + void addInductionPhi(PHINode *Phi, InductionDescriptor ID,<br>
> + SmallPtrSetImpl<Value *> &AllowedExit);<br>
><br>
> /// Report an analysis message to assist the user in diagnosing loops that are<br>
> /// not vectorized. These are handled as LoopAccessReport rather than<br>
> @@ -1493,7 +1497,7 @@ private:<br>
> /// Holds the widest induction type encountered.<br>
> Type *WidestIndTy;<br>
><br>
> - /// Allowed outside users. This holds the reduction<br>
> + /// Allowed outside users. This holds the induction and reduction<br>
> /// vars which can be accessed from outside the loop.<br>
> SmallPtrSet<Value *, 4> AllowedExit;<br>
> /// This set holds the variables which are known to be uniform after<br>
> @@ -3219,6 +3223,9 @@ void InnerLoopVectorizer::createEmptyLoo<br>
> // or the value at the end of the vectorized loop.<br>
> BCResumeVal->addIncoming(EndValue, MiddleBlock);<br>
><br>
> + // Fix up external users of the induction variable.<br>
> + fixupIVUsers(OrigPhi, II, CountRoundDown, EndValue, MiddleBlock);<br>
> +<br>
> // Fix the scalar body counter (PHI node).<br>
> unsigned BlockIdx = OrigPhi->getBasicBlockIndex(ScalarPH);<br>
><br>
> @@ -3258,6 +3265,59 @@ void InnerLoopVectorizer::createEmptyLoo<br>
> Hints.setAlreadyVectorized();<br>
> }<br>
><br>
> +// Fix up external users of the induction variable. At this point, we are<br>
> +// in LCSSA form, with all external PHIs that use the IV having one input value,<br>
> +// coming from the remainder loop. We need those PHIs to also have a correct<br>
> +// value for the IV when arriving directly from the middle block.<br>
> +void InnerLoopVectorizer::fixupIVUsers(PHINode *OrigPhi,<br>
> + const InductionDescriptor &II,<br>
> + Value *CountRoundDown, Value *EndValue,<br>
> + BasicBlock *MiddleBlock) {<br>
> + // There are two kinds of external IV usages - those that use the value<br>
> + // computed in the last iteration (the PHI) and those that use the penultimate<br>
> + // value (the value that feeds into the phi from the loop latch).<br>
> + // We allow both, but they, obviously, have different values.<br>
> +<br>
> + // We only expect at most one of each kind of user. This is because LCSSA will<br>
> + // canonicalize the users to a single PHI node per exit block, and we<br>
> + // currently only vectorize loops with a single exit.<br>
> + assert(OrigLoop->getExitBlock() && "Expected a single exit block");<br>
> +<br>
> + // An external user of the last iteration's value should see the value that<br>
> + // the remainder loop uses to initialize its own IV.<br>
> + Value *PostInc = OrigPhi->getIncomingValueForBlock(OrigLoop->getLoopLatch());<br>
> + for (User *U : PostInc->users()) {<br>
> + Instruction *UI = cast<Instruction>(U);<br>
> + if (!OrigLoop->contains(UI)) {<br>
> + assert(isa<PHINode>(UI) && "Expected LCSSA form");<br>
> + cast<PHINode>(UI)->addIncoming(EndValue, MiddleBlock);<br>
> + break;<br>
> + }<br>
> + }<br>
> +<br>
> + // An external user of the penultimate value need to see EndValue - Step.<br>
> + // The simplest way to get this is to recompute it from the constituent SCEVs,<br>
> + // that is Start + (Step * (CRD - 1)).<br>
> + for (User *U : OrigPhi->users()) {<br>
> + Instruction *UI = cast<Instruction>(U);<br>
> + if (!OrigLoop->contains(UI)) {<br>
> + assert(isa<PHINode>(UI) && "Expected LCSSA form");<br>
> + const DataLayout &DL =<br>
> + OrigLoop->getHeader()->getModule()->getDataLayout();<br>
> +<br>
> + IRBuilder<> B(MiddleBlock->getTerminator());<br>
> + Value *CountMinusOne = B.CreateSub(<br>
> + CountRoundDown, ConstantInt::get(CountRoundDown->getType(), 1));<br>
> + Value *CMO = B.CreateSExtOrTrunc(CountMinusOne, II.getStep()->getType(),<br>
> + "cast.cmo");<br>
> + Value *Escape = II.transform(B, CMO, PSE.getSE(), DL);<br>
> + Escape->setName("ind.escape");<br>
> + cast<PHINode>(UI)->addIncoming(Escape, MiddleBlock);<br>
> + break;<br>
> + }<br>
> + }<br>
> +}<br>
> +<br>
> namespace {<br>
> struct CSEDenseMapInfo {<br>
> static bool canHandle(Instruction *I) {<br>
> @@ -4639,10 +4699,10 @@ static Type *getWiderType(const DataLayo<br>
> /// \brief Check that the instruction has outside loop users and is not an<br>
> /// identified reduction variable.<br>
> static bool hasOutsideLoopUser(const Loop *TheLoop, Instruction *Inst,<br>
> - SmallPtrSetImpl<Value *> &Reductions) {<br>
> - // Reduction instructions are allowed to have exit users. All other<br>
> - // instructions must not have external users.<br>
> - if (!Reductions.count(Inst))<br>
> + SmallPtrSetImpl<Value *> &AllowedExit) {<br>
> + // Reduction and Induction instructions are allowed to have exit users. All<br>
> + // other instructions must not have external users.<br>
> + if (!AllowedExit.count(Inst))<br>
> // Check that all of the users of the loop are inside the BB.<br>
> for (User *U : Inst->users()) {<br>
> Instruction *UI = cast<Instruction>(U);<br>
> @@ -4655,8 +4715,9 @@ static bool hasOutsideLoopUser(const Loo<br>
> return false;<br>
> }<br>
><br>
> -bool LoopVectorizationLegality::addInductionPhi(PHINode *Phi,<br>
> - InductionDescriptor ID) {<br>
> +void LoopVectorizationLegality::addInductionPhi(<br>
> + PHINode *Phi, InductionDescriptor ID,<br>
> + SmallPtrSetImpl<Value *> &AllowedExit) {<br>
> Inductions[Phi] = ID;<br>
> Type *PhiTy = Phi->getType();<br>
> const DataLayout &DL = Phi->getModule()->getDataLayout();<br>
> @@ -4682,18 +4743,13 @@ bool LoopVectorizationLegality::addInduc<br>
> Induction = Phi;<br>
> }<br>
><br>
> - DEBUG(dbgs() << "LV: Found an induction variable.\n");<br>
> + // Both the PHI node itself, and the "post-increment" value feeding<br>
> + // back into the PHI node may have external users.<br>
> + AllowedExit.insert(Phi);<br>
> + AllowedExit.insert(Phi->getIncomingValueForBlock(TheLoop->getLoopLatch()));<br>
><br>
> - // Until we explicitly handle the case of an induction variable with<br>
> - // an outside loop user we have to give up vectorizing this loop.<br>
> - if (hasOutsideLoopUser(TheLoop, Phi, AllowedExit)) {<br>
> - emitAnalysis(VectorizationReport(Phi) <<<br>
> - "use of induction value outside of the "<br>
> - "loop is not handled by vectorizer");<br>
> - return false;<br>
> - }<br>
> -<br>
> - return true;<br>
> + DEBUG(dbgs() << "LV: Found an induction variable.\n");<br>
> + return;<br>
> }<br>
><br>
> bool LoopVectorizationLegality::canVectorizeInstrs() {<br>
> @@ -4757,8 +4813,7 @@ bool LoopVectorizationLegality::canVecto<br>
><br>
> InductionDescriptor ID;<br>
> if (InductionDescriptor::isInductionPHI(Phi, PSE, ID)) {<br>
> - if (!addInductionPhi(Phi, ID))<br>
> - return false;<br>
> + addInductionPhi(Phi, ID, AllowedExit);<br>
> continue;<br>
> }<br>
><br>
> @@ -4770,8 +4825,7 @@ bool LoopVectorizationLegality::canVecto<br>
> // As a last resort, coerce the PHI to a AddRec expression<br>
> // and re-try classifying it a an induction PHI.<br>
> if (InductionDescriptor::isInductionPHI(Phi, PSE, ID, true)) {<br>
> - if (!addInductionPhi(Phi, ID))<br>
> - return false;<br>
> + addInductionPhi(Phi, ID, AllowedExit);<br>
> continue;<br>
> }<br>
><br>
><br>
> Added: llvm/trunk/test/Transforms/LoopVectorize/iv_outside_user.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/iv_outside_user.ll?rev=272715&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/iv_outside_user.ll?rev=272715&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/Transforms/LoopVectorize/iv_outside_user.ll (added)<br>
> +++ llvm/trunk/test/Transforms/LoopVectorize/iv_outside_user.ll Tue Jun 14 16:27:27 2016<br>
> @@ -0,0 +1,84 @@<br>
> +; RUN: opt -S -loop-vectorize -force-vector-interleave=1 -force-vector-width=2 < %s | FileCheck %s<br>
> +<br>
> +; CHECK-LABEL: @postinc<br>
> +; CHECK-LABEL: <a href="http://scalar.ph" rel="noreferrer" target="_blank">scalar.ph</a>:<br>
> +; CHECK: %bc.resume.val = phi i32 [ %n.vec, %middle.block ], [ 0, %entry ]<br>
> +; CHECK-LABEL: for.end:<br>
> +; CHECK: %[[RET:.*]] = phi i32 [ {{.*}}, %for.body ], [ %n.vec, %middle.block ]<br>
> +; CHECK: ret i32 %[[RET]]<br>
> +define i32 @postinc(i32 %k) {<br>
> +entry:<br>
> + br label %for.body<br>
> +<br>
> +for.body:<br>
> + %inc.phi = phi i32 [ 0, %entry ], [ %inc, %for.body ]<br>
> + %inc = add nsw i32 %inc.phi, 1<br>
> + %cmp = icmp eq i32 %inc, %k<br>
> + br i1 %cmp, label %for.end, label %for.body<br>
> +<br>
> +for.end:<br>
> + ret i32 %inc<br>
> +}<br>
> +<br>
> +; CHECK-LABEL: @preinc<br>
> +; CHECK-LABEL: middle.block:<br>
> +; CHECK: %3 = sub i32 %n.vec, 1<br>
> +; CHECK: %ind.escape = add i32 0, %3<br>
> +; CHECK-LABEL: <a href="http://scalar.ph" rel="noreferrer" target="_blank">scalar.ph</a>:<br>
> +; CHECK: %bc.resume.val = phi i32 [ %n.vec, %middle.block ], [ 0, %entry ]<br>
> +; CHECK-LABEL: for.end:<br>
> +; CHECK: %[[RET:.*]] = phi i32 [ {{.*}}, %for.body ], [ %ind.escape, %middle.block ]<br>
> +; CHECK: ret i32 %[[RET]]<br>
> +define i32 @preinc(i32 %k) {<br>
> +entry:<br>
> + br label %for.body<br>
> +<br>
> +for.body:<br>
> + %inc.phi = phi i32 [ 0, %entry ], [ %inc, %for.body ]<br>
> + %inc = add nsw i32 %inc.phi, 1<br>
> + %cmp = icmp eq i32 %inc, %k<br>
> + br i1 %cmp, label %for.end, label %for.body<br>
> +<br>
> +for.end:<br>
> + ret i32 %inc.phi<br>
> +}<br>
> +<br>
> +; CHECK-LABEL: @constpre<br>
> +; CHECK-LABEL: for.end:<br>
> +; CHECK: %[[RET:.*]] = phi i32 [ {{.*}}, %for.body ], [ 2, %middle.block ]<br>
> +; CHECK: ret i32 %[[RET]]<br>
> +define i32 @constpre() {<br>
> +entry:<br>
> + br label %for.body<br>
> +<br>
> +for.body:<br>
> + %inc.phi = phi i32 [ 32, %entry ], [ %inc, %for.body ]<br>
> + %inc = sub nsw i32 %inc.phi, 2<br>
> + %cmp = icmp eq i32 %inc, 0<br>
> + br i1 %cmp, label %for.end, label %for.body<br>
> +<br>
> +for.end:<br>
> + ret i32 %inc.phi<br>
> +}<br>
> +<br>
> +; CHECK-LABEL: @geppre<br>
> +; CHECK-LABEL: middle.block:<br>
> +; CHECK: %ind.escape = getelementptr i32, i32* %ptr, i64 124<br>
> +; CHECK-LABEL: for.end:<br>
> +; CHECK: %[[RET:.*]] = phi i32* [ {{.*}}, %for.body ], [ %ind.escape, %middle.block ]<br>
> +; CHECK: ret i32* %[[RET]]<br>
> +define i32* @geppre(i32* %ptr) {<br>
> +entry:<br>
> + br label %for.body<br>
> +<br>
> +for.body:<br>
> + %inc.phi = phi i32 [ 0, %entry ], [ %inc, %for.body ]<br>
> + %ptr.phi = phi i32* [ %ptr, %entry ], [ %inc.ptr, %for.body ]<br>
> + %inc = add nsw i32 %inc.phi, 1<br>
> + %inc.ptr = getelementptr i32, i32* %ptr.phi, i32 4<br>
> + %cmp = icmp eq i32 %inc, 32<br>
> + br i1 %cmp, label %for.end, label %for.body<br>
> +<br>
> +for.end:<br>
> + ret i32* %ptr.phi<br>
> +}<br>
><br>
> Modified: llvm/trunk/test/Transforms/LoopVectorize/no_outside_user.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/no_outside_user.ll?rev=272715&r1=272714&r2=272715&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/no_outside_user.ll?rev=272715&r1=272714&r2=272715&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/Transforms/LoopVectorize/no_outside_user.ll (original)<br>
> +++ llvm/trunk/test/Transforms/LoopVectorize/no_outside_user.ll Tue Jun 14 16:27:27 2016<br>
> @@ -1,7 +1,6 @@<br>
> ; RUN: opt -S -loop-vectorize -force-vector-interleave=1 -force-vector-width=2 < %s 2>&1 | FileCheck %s<br>
><br>
> ; CHECK: remark: {{.*}}: loop not vectorized: value could not be identified as an induction or reduction variable<br>
> -; CHECK: remark: {{.*}}: loop not vectorized: use of induction value outside of the loop is not handled by vectorizer<br>
><br>
> target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:32:32-n8:16:32-S128"<br>
><br>
> @@ -41,34 +40,3 @@ f1.exit.loopexit:<br>
> %.lcssa = phi i32 [ %tmp17, %bb16 ]<br>
> ret i32 %.lcssa<br>
> }<br>
> -<br>
> -; Don't vectorize this loop. Its phi node (induction variable) has an outside<br>
> -; loop user. We currently don't handle this case.<br>
> -; PR17179<br>
> -<br>
> -; CHECK-LABEL: @test2(<br>
> -; CHECK-NOT: <2 x<br>
> -<br>
> -@x1 = common global i32 0, align 4<br>
> -@x2 = common global i32 0, align 4<br>
> -@x0 = common global i32 0, align 4<br>
> -<br>
> -define i32 @test2() {<br>
> -entry:<br>
> - store i32 0, i32* @x1, align 4<br>
> - %0 = load i32, i32* @x0, align 4<br>
> - br label %for.cond1.preheader<br>
> -<br>
> -for.cond1.preheader:<br>
> - %inc7 = phi i32 [ 0, %entry ], [ %inc, %for.cond1.preheader ]<br>
> - %inc = add nsw i32 %inc7, 1<br>
> - %cmp = icmp eq i32 %inc, 52<br>
> - br i1 %cmp, label %for.end5, label %for.cond1.preheader<br>
> -<br>
> -for.end5:<br>
> - %inc7.lcssa = phi i32 [ %inc7, %for.cond1.preheader ]<br>
> - %xor = xor i32 %inc7.lcssa, %0<br>
> - store i32 52, i32* @x1, align 4<br>
> - store i32 1, i32* @x2, align 4<br>
> - ret i32 %xor<br>
> -}<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>