[llvm] r295957 - Revert "[SLP] Fix for PR32036: Vectorized horizontal reduction returning wrong"
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 24 18:40:25 PST 2017
This change was applied and reverted multiple times without comment.
Our community expectation is that if you revert a change, you explain
why. This can either be in the revert commit message itself or in an
email response to the original commit (preferred).
Philip
On 02/23/2017 03:09 AM, Alexey Bataev via llvm-commits wrote:
> Author: abataev
> Date: Thu Feb 23 05:09:35 2017
> New Revision: 295957
>
> URL: http://llvm.org/viewvc/llvm-project?rev=295957&view=rev
> Log:
> Revert "[SLP] Fix for PR32036: Vectorized horizontal reduction returning wrong"
>
> This reverts commit 7c5141e577d9efd1c8e3087566a38ce6b3a41a84.
>
> Modified:
> llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp
> llvm/trunk/test/Transforms/SLPVectorizer/X86/horizontal-list.ll
>
> Modified: llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp?rev=295957&r1=295956&r2=295957&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp (original)
> +++ llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp Thu Feb 23 05:09:35 2017
> @@ -304,7 +304,6 @@ public:
> typedef SmallVector<Instruction *, 16> InstrList;
> typedef SmallPtrSet<Value *, 16> ValueSet;
> typedef SmallVector<StoreInst *, 8> StoreList;
> - typedef MapVector<Value *, SmallVector<DebugLoc, 4>> ExtraValueToDebugLocsMap;
>
> BoUpSLP(Function *Func, ScalarEvolution *Se, TargetTransformInfo *Tti,
> TargetLibraryInfo *TLi, AliasAnalysis *Aa, LoopInfo *Li,
> @@ -334,7 +333,7 @@ public:
> /// Vectorize the tree but with the list of externally used values \p
> /// ExternallyUsedValues. Values in this MapVector can be replaced but the
> /// generated extractvalue instructions.
> - Value *vectorizeTree(ExtraValueToDebugLocsMap &ExternallyUsedValues);
> + Value *vectorizeTree(MapVector<Value *, DebugLoc> &ExternallyUsedValues);
>
> /// \returns the cost incurred by unwanted spills and fills, caused by
> /// holding live values over call sites.
> @@ -353,7 +352,7 @@ public:
> /// into account (anf updating it, if required) list of externally used
> /// values stored in \p ExternallyUsedValues.
> void buildTree(ArrayRef<Value *> Roots,
> - ExtraValueToDebugLocsMap &ExternallyUsedValues,
> + MapVector<Value *, DebugLoc> &ExternallyUsedValues,
> ArrayRef<Value *> UserIgnoreLst = None);
>
> /// Clear the internal data structures that are created by 'buildTree'.
> @@ -954,11 +953,11 @@ private:
>
> void BoUpSLP::buildTree(ArrayRef<Value *> Roots,
> ArrayRef<Value *> UserIgnoreLst) {
> - ExtraValueToDebugLocsMap ExternallyUsedValues;
> + MapVector<Value *, DebugLoc> ExternallyUsedValues;
> buildTree(Roots, ExternallyUsedValues, UserIgnoreLst);
> }
> void BoUpSLP::buildTree(ArrayRef<Value *> Roots,
> - ExtraValueToDebugLocsMap &ExternallyUsedValues,
> + MapVector<Value *, DebugLoc> &ExternallyUsedValues,
> ArrayRef<Value *> UserIgnoreLst) {
> deleteTree();
> UserIgnoreList = UserIgnoreLst;
> @@ -2802,12 +2801,12 @@ Value *BoUpSLP::vectorizeTree(ArrayRef<V
> }
>
> Value *BoUpSLP::vectorizeTree() {
> - ExtraValueToDebugLocsMap ExternallyUsedValues;
> + MapVector<Value *, DebugLoc> ExternallyUsedValues;
> return vectorizeTree(ExternallyUsedValues);
> }
>
> Value *
> -BoUpSLP::vectorizeTree(ExtraValueToDebugLocsMap &ExternallyUsedValues) {
> +BoUpSLP::vectorizeTree(MapVector<Value *, DebugLoc> &ExternallyUsedValues) {
>
> // All blocks must be scheduled before any instructions are inserted.
> for (auto &BSIter : BlocksSchedules) {
> @@ -2869,6 +2868,7 @@ BoUpSLP::vectorizeTree(ExtraValueToDebug
> assert(ExternallyUsedValues.count(Scalar) &&
> "Scalar with nullptr as an external user must be registered in "
> "ExternallyUsedValues map");
> + DebugLoc DL = ExternallyUsedValues[Scalar];
> if (auto *VecI = dyn_cast<Instruction>(Vec)) {
> Builder.SetInsertPoint(VecI->getParent(),
> std::next(VecI->getIterator()));
> @@ -2878,8 +2878,8 @@ BoUpSLP::vectorizeTree(ExtraValueToDebug
> Value *Ex = Builder.CreateExtractElement(Vec, Lane);
> Ex = extend(ScalarRoot, Ex, Scalar->getType());
> CSEBlocks.insert(cast<Instruction>(Scalar)->getParent());
> - std::swap(ExternallyUsedValues[Ex], ExternallyUsedValues[Scalar]);
> - assert(ExternallyUsedValues[Scalar].empty());
> + ExternallyUsedValues.erase(Scalar);
> + ExternallyUsedValues[Ex] = DL;
> continue;
> }
>
> @@ -4439,11 +4439,9 @@ public:
> Builder.setFastMathFlags(Unsafe);
> unsigned i = 0;
>
> - BoUpSLP::ExtraValueToDebugLocsMap ExternallyUsedValues;
> - // The same extra argument may be used several time, so log each attempt
> - // to use it.
> + MapVector<Value *, DebugLoc> ExternallyUsedValues;
> for (auto &Pair : ExtraArgs)
> - ExternallyUsedValues[Pair.second].push_back(Pair.first->getDebugLoc());
> + ExternallyUsedValues[Pair.second] = Pair.first->getDebugLoc();
> while (i < NumReducedVals - ReduxWidth + 1 && ReduxWidth > 2) {
> auto VL = makeArrayRef(&ReducedVals[i], ReduxWidth);
> V.buildTree(VL, ExternallyUsedValues, ReductionOps);
> @@ -4491,14 +4489,9 @@ public:
> Builder.CreateBinOp(ReductionOpcode, VectorizedTree, I);
> }
> for (auto &Pair : ExternallyUsedValues) {
> - if (Pair.second.empty())
> - continue;
> - // Add each externally used value to the final reduction.
> - for (auto &DL : Pair.second) {
> - Builder.SetCurrentDebugLocation(DL);
> - VectorizedTree = Builder.CreateBinOp(ReductionOpcode, VectorizedTree,
> - Pair.first, "bin.extra");
> - }
> + Builder.SetCurrentDebugLocation(Pair.second);
> + VectorizedTree = Builder.CreateBinOp(ReductionOpcode, VectorizedTree,
> + Pair.first, "bin.extra");
> }
> // Update users.
> if (ReductionPHI && !isa<UndefValue>(ReductionPHI)) {
>
> Modified: llvm/trunk/test/Transforms/SLPVectorizer/X86/horizontal-list.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SLPVectorizer/X86/horizontal-list.ll?rev=295957&r1=295956&r2=295957&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/SLPVectorizer/X86/horizontal-list.ll (original)
> +++ llvm/trunk/test/Transforms/SLPVectorizer/X86/horizontal-list.ll Thu Feb 23 05:09:35 2017
> @@ -1473,10 +1473,9 @@ define float @extra_args_same_several_ti
> ; CHECK-NEXT: [[TMP2:%.*]] = extractelement <8 x float> [[BIN_RDX4]], i32 0
> ; CHECK-NEXT: [[BIN_EXTRA:%.*]] = fadd fast float [[TMP2]], [[ADD]]
> ; CHECK-NEXT: [[BIN_EXTRA5:%.*]] = fadd fast float [[BIN_EXTRA]], 5.000000e+00
> -; CHECK-NEXT: [[BIN_EXTRA6:%.*]] = fadd fast float [[BIN_EXTRA5]], 5.000000e+00
> -; CHECK-NEXT: [[BIN_EXTRA7:%.*]] = fadd fast float [[BIN_EXTRA6]], [[CONV]]
> +; CHECK-NEXT: [[BIN_EXTRA6:%.*]] = fadd fast float [[BIN_EXTRA5]], [[CONV]]
> ; CHECK-NEXT: [[ADD4_6:%.*]] = fadd fast float undef, [[ADD4_5]]
> -; CHECK-NEXT: ret float [[BIN_EXTRA7]]
> +; CHECK-NEXT: ret float [[BIN_EXTRA6]]
> ;
> ; THRESHOLD-LABEL: @extra_args_same_several_times(
> ; THRESHOLD-NEXT: entry:
> @@ -1511,10 +1510,9 @@ define float @extra_args_same_several_ti
> ; THRESHOLD-NEXT: [[TMP2:%.*]] = extractelement <8 x float> [[BIN_RDX4]], i32 0
> ; THRESHOLD-NEXT: [[BIN_EXTRA:%.*]] = fadd fast float [[TMP2]], [[ADD]]
> ; THRESHOLD-NEXT: [[BIN_EXTRA5:%.*]] = fadd fast float [[BIN_EXTRA]], 5.000000e+00
> -; THRESHOLD-NEXT: [[BIN_EXTRA6:%.*]] = fadd fast float [[BIN_EXTRA5]], 5.000000e+00
> -; THRESHOLD-NEXT: [[BIN_EXTRA7:%.*]] = fadd fast float [[BIN_EXTRA6]], [[CONV]]
> +; THRESHOLD-NEXT: [[BIN_EXTRA6:%.*]] = fadd fast float [[BIN_EXTRA5]], [[CONV]]
> ; THRESHOLD-NEXT: [[ADD4_6:%.*]] = fadd fast float undef, [[ADD4_5]]
> -; THRESHOLD-NEXT: ret float [[BIN_EXTRA7]]
> +; THRESHOLD-NEXT: ret float [[BIN_EXTRA6]]
> ;
> entry:
> %mul = mul nsw i32 %b, %a
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list