[llvm] r207746 - Revert r205965, which essentially reverts r205018 for the second time.
Chandler Carruth
chandlerc at gmail.com
Thu May 1 11:10:03 PDT 2014
My suspicion is that after debugging, Arnold will end up with an even
cleaner testcase. I figured he would check in whatever he ended up with.
On May 1, 2014 7:01 AM, "Rafael Espíndola" <rafael.espindola at gmail.com>
wrote:
> what about checking in the testcase in PR19621?
>
> On 1 May 2014 07:24, Chandler Carruth <chandlerc at gmail.com> wrote:
> > Author: chandlerc
> > Date: Thu May 1 06:24:11 2014
> > New Revision: 207746
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=207746&view=rev
> > Log:
> > Revert r205965, which essentially reverts r205018 for the second time.
> > =[
> >
> > Turns out that this was the root cause of PR19621. We found a crasher
> > only recently (likely due to improvements elsewhere in the SLP
> > vectorizer) but the reduced test case failed all the way back to here.
> > I've confirmed that reverting this patch both fixes the reduced test
> > case in PR19621 and the actual source file that led to it, so it seems
> > to really be rooted here. I've replied to the commit thread with
> > discussion of my (feeble) attempts to debug this. Didn't make it very
> > far, so reverting now that we have a good test case so that things can
> > get back to healthy while the debugging carries on.
> >
> > Modified:
> > llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp
> >
> llvm/trunk/test/Transforms/SLPVectorizer/X86/insert-element-build-vector.ll
> >
> > Modified: llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp?rev=207746&r1=207745&r2=207746&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp Thu May 1
> 06:24:11 2014
> > @@ -366,13 +366,13 @@ public:
> > /// A negative number means that this is profitable.
> > int getTreeCost();
> >
> > - /// Construct a vectorizable tree that starts at \p Roots, ignoring
> users for
> > - /// the purpose of scheduling and extraction in the \p UserIgnoreLst.
> > - void buildTree(ArrayRef<Value *> Roots,
> > - ArrayRef<Value *> UserIgnoreLst = None);
> > + /// Construct a vectorizable tree that starts at \p Roots and is
> possibly
> > + /// used by a reduction of \p RdxOps.
> > + void buildTree(ArrayRef<Value *> Roots, ValueSet *RdxOps = 0);
> >
> > /// Clear the internal data structures that are created by
> 'buildTree'.
> > void deleteTree() {
> > + RdxOps = 0;
> > VectorizableTree.clear();
> > ScalarToTreeEntry.clear();
> > MustGather.clear();
> > @@ -528,8 +528,8 @@ private:
> > /// Numbers instructions in different blocks.
> > DenseMap<BasicBlock *, BlockNumbering> BlocksNumbers;
> >
> > - /// List of users to ignore during scheduling and that don't need
> extracting.
> > - ArrayRef<Value *> UserIgnoreList;
> > + /// Reduction operators.
> > + ValueSet *RdxOps;
> >
> > // Analysis and block reference.
> > Function *F;
> > @@ -543,10 +543,9 @@ private:
> > IRBuilder<> Builder;
> > };
> >
> > -void BoUpSLP::buildTree(ArrayRef<Value *> Roots,
> > - ArrayRef<Value *> UserIgnoreLst) {
> > +void BoUpSLP::buildTree(ArrayRef<Value *> Roots, ValueSet *Rdx) {
> > deleteTree();
> > - UserIgnoreList = UserIgnoreLst;
> > + RdxOps = Rdx;
> > if (!getSameType(Roots))
> > return;
> > buildTree_rec(Roots, 0);
> > @@ -578,9 +577,8 @@ void BoUpSLP::buildTree(ArrayRef<Value *
> > if (!UserInst)
> > continue;
> >
> > - // Ignore users in the user ignore list.
> > - if (std::find(UserIgnoreList.begin(), UserIgnoreList.end(),
> UserInst) !=
> > - UserIgnoreList.end())
> > + // Ignore uses that are part of the reduction.
> > + if (Rdx && std::find(Rdx->begin(), Rdx->end(), UserInst) !=
> Rdx->end())
> > continue;
> >
> > DEBUG(dbgs() << "SLP: Need to extract:" << *U << " from lane "
> <<
> > @@ -711,9 +709,8 @@ void BoUpSLP::buildTree_rec(ArrayRef<Val
> > continue;
> > }
> >
> > - // Ignore users in the user ignore list.
> > - if (std::find(UserIgnoreList.begin(), UserIgnoreList.end(), UI) !=
> > - UserIgnoreList.end())
> > + // This user is part of the reduction.
> > + if (RdxOps && RdxOps->count(UI))
> > continue;
> >
> > // Make sure that we can schedule this unknown user.
> > @@ -1752,9 +1749,8 @@ Value *BoUpSLP::vectorizeTree() {
> > DEBUG(dbgs() << "SLP: \tvalidating user:" << *U << ".\n");
> >
> > assert((ScalarToTreeEntry.count(U) ||
> > - // It is legal to replace users in the ignorelist by
> undef.
> > - (std::find(UserIgnoreList.begin(),
> UserIgnoreList.end(), U) !=
> > - UserIgnoreList.end())) &&
> > + // It is legal to replace the reduction users by
> undef.
> > + (RdxOps && RdxOps->count(U))) &&
> > "Replacing out-of-tree value with undef");
> > }
> > #endif
> > @@ -1958,11 +1954,8 @@ private:
> > bool tryToVectorizePair(Value *A, Value *B, BoUpSLP &R);
> >
> > /// \brief Try to vectorize a list of operands.
> > - /// \@param BuildVector A list of users to ignore for the purpose of
> > - /// scheduling and that don't need extracting.
> > /// \returns true if a value was vectorized.
> > - bool tryToVectorizeList(ArrayRef<Value *> VL, BoUpSLP &R,
> > - ArrayRef<Value *> BuildVector = None);
> > + bool tryToVectorizeList(ArrayRef<Value *> VL, BoUpSLP &R);
> >
> > /// \brief Try to vectorize a chain that may start at the operands of
> \V;
> > bool tryToVectorize(BinaryOperator *V, BoUpSLP &R);
> > @@ -2135,8 +2128,7 @@ bool SLPVectorizer::tryToVectorizePair(V
> > return tryToVectorizeList(VL, R);
> > }
> >
> > -bool SLPVectorizer::tryToVectorizeList(ArrayRef<Value *> VL, BoUpSLP &R,
> > - ArrayRef<Value *> BuildVector) {
> > +bool SLPVectorizer::tryToVectorizeList(ArrayRef<Value *> VL, BoUpSLP
> &R) {
> > if (VL.size() < 2)
> > return false;
> >
> > @@ -2186,33 +2178,13 @@ bool SLPVectorizer::tryToVectorizeList(A
> > << "\n");
> > ArrayRef<Value *> Ops = VL.slice(i, OpsWidth);
> >
> > - ArrayRef<Value *> BuildVectorSlice;
> > - if (!BuildVector.empty())
> > - BuildVectorSlice = BuildVector.slice(i, OpsWidth);
> > -
> > - R.buildTree(Ops, BuildVectorSlice);
> > + R.buildTree(Ops);
> > int Cost = R.getTreeCost();
> >
> > if (Cost < -SLPCostThreshold) {
> > DEBUG(dbgs() << "SLP: Vectorizing list at cost:" << Cost <<
> ".\n");
> > - Value *VectorizedRoot = R.vectorizeTree();
> > + R.vectorizeTree();
> >
> > - // Reconstruct the build vector by extracting the vectorized
> root. This
> > - // way we handle the case where some elements of the vector are
> undefined.
> > - // (return (inserelt <4 xi32> (insertelt undef (opd0) 0) (opd1)
> 2))
> > - if (!BuildVectorSlice.empty()) {
> > - Instruction *InsertAfter = cast<Instruction>(VectorizedRoot);
> > - for (auto &V : BuildVectorSlice) {
> > - InsertElementInst *IE = cast<InsertElementInst>(V);
> > - IRBuilder<> Builder(++BasicBlock::iterator(InsertAfter));
> > - Instruction *Extract = cast<Instruction>(
> > - Builder.CreateExtractElement(VectorizedRoot,
> IE->getOperand(2)));
> > - IE->setOperand(1, Extract);
> > - IE->removeFromParent();
> > - IE->insertAfter(Extract);
> > - InsertAfter = IE;
> > - }
> > - }
> > // Move to the next bundle.
> > i += VF - 1;
> > Changed = true;
> > @@ -2321,7 +2293,7 @@ static Value *createRdxShuffleMask(unsig
> > /// *p =
> > ///
> > class HorizontalReduction {
> > - SmallVector<Value *, 16> ReductionOps;
> > + SmallPtrSet<Value *, 16> ReductionOps;
> > SmallVector<Value *, 32> ReducedVals;
> >
> > BinaryOperator *ReductionRoot;
> > @@ -2415,7 +2387,7 @@ public:
> > // We need to be able to reassociate the adds.
> > if (!TreeN->isAssociative())
> > return false;
> > - ReductionOps.push_back(TreeN);
> > + ReductionOps.insert(TreeN);
> > }
> > // Retract.
> > Stack.pop_back();
> > @@ -2452,7 +2424,7 @@ public:
> >
> > for (; i < NumReducedVals - ReduxWidth + 1; i += ReduxWidth) {
> > ArrayRef<Value *> ValsToReduce(&ReducedVals[i], ReduxWidth);
> > - V.buildTree(ValsToReduce, ReductionOps);
> > + V.buildTree(ValsToReduce, &ReductionOps);
> >
> > // Estimate cost.
> > int Cost = V.getTreeCost() + getReductionCost(TTI,
> ReducedVals[i]);
> > @@ -2571,16 +2543,13 @@ private:
> > ///
> > /// Returns true if it matches
> > ///
> > -static bool findBuildVector(InsertElementInst *FirstInsertElem,
> > - SmallVectorImpl<Value *> &BuildVector,
> > - SmallVectorImpl<Value *> &BuildVectorOpds) {
> > - if (!isa<UndefValue>(FirstInsertElem->getOperand(0)))
> > +static bool findBuildVector(InsertElementInst *IE,
> > + SmallVectorImpl<Value *> &Ops) {
> > + if (!isa<UndefValue>(IE->getOperand(0)))
> > return false;
> >
> > - InsertElementInst *IE = FirstInsertElem;
> > while (true) {
> > - BuildVector.push_back(IE);
> > - BuildVectorOpds.push_back(IE->getOperand(1));
> > + Ops.push_back(IE->getOperand(1));
> >
> > if (IE->use_empty())
> > return false;
> > @@ -2751,16 +2720,12 @@ bool SLPVectorizer::vectorizeChainsInBlo
> > }
> >
> > // Try to vectorize trees that start at insertelement instructions.
> > - if (InsertElementInst *FirstInsertElem =
> dyn_cast<InsertElementInst>(it)) {
> > - SmallVector<Value *, 16> BuildVector;
> > - SmallVector<Value *, 16> BuildVectorOpds;
> > - if (!findBuildVector(FirstInsertElem, BuildVector,
> BuildVectorOpds))
> > + if (InsertElementInst *IE = dyn_cast<InsertElementInst>(it)) {
> > + SmallVector<Value *, 8> Ops;
> > + if (!findBuildVector(IE, Ops))
> > continue;
> >
> > - // Vectorize starting with the build vector operands ignoring the
> > - // BuildVector instructions for the purpose of scheduling and user
> > - // extraction.
> > - if (tryToVectorizeList(BuildVectorOpds, R, BuildVector)) {
> > + if (tryToVectorizeList(Ops, R)) {
> > Changed = true;
> > it = BB->begin();
> > e = BB->end();
> >
> > Modified:
> llvm/trunk/test/Transforms/SLPVectorizer/X86/insert-element-build-vector.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SLPVectorizer/X86/insert-element-build-vector.ll?rev=207746&r1=207745&r2=207746&view=diff
> >
> ==============================================================================
> > ---
> llvm/trunk/test/Transforms/SLPVectorizer/X86/insert-element-build-vector.ll
> (original)
> > +++
> llvm/trunk/test/Transforms/SLPVectorizer/X86/insert-element-build-vector.ll
> Thu May 1 06:24:11 2014
> > @@ -195,30 +195,6 @@ define <4 x float> @simple_select_partia
> > ret <4 x float> %rb
> > }
> >
> > -; Make sure that vectorization happens even if insertelements operations
> > -; must be rescheduled. The case here is from compiling Julia.
> > -define <4 x float> @reschedule_extract(<4 x float> %a, <4 x float> %b) {
> > -; CHECK-LABEL: @reschedule_extract(
> > -; CHECK: %1 = fadd <4 x float> %a, %b
> > - %a0 = extractelement <4 x float> %a, i32 0
> > - %b0 = extractelement <4 x float> %b, i32 0
> > - %c0 = fadd float %a0, %b0
> > - %v0 = insertelement <4 x float> undef, float %c0, i32 0
> > - %a1 = extractelement <4 x float> %a, i32 1
> > - %b1 = extractelement <4 x float> %b, i32 1
> > - %c1 = fadd float %a1, %b1
> > - %v1 = insertelement <4 x float> %v0, float %c1, i32 1
> > - %a2 = extractelement <4 x float> %a, i32 2
> > - %b2 = extractelement <4 x float> %b, i32 2
> > - %c2 = fadd float %a2, %b2
> > - %v2 = insertelement <4 x float> %v1, float %c2, i32 2
> > - %a3 = extractelement <4 x float> %a, i32 3
> > - %b3 = extractelement <4 x float> %b, i32 3
> > - %c3 = fadd float %a3, %b3
> > - %v3 = insertelement <4 x float> %v2, float %c3, i32 3
> > - ret <4 x float> %v3
> > -}
> > -
> > ; Check that cost model for vectorization takes credit for
> > ; instructions that are erased.
> > define <4 x float> @take_credit(<4 x float> %a, <4 x float> %b) {
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140501/58ef7449/attachment.html>
More information about the llvm-commits
mailing list