[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