[llvm] r295956 - [SLP] Fix for PR32036: Vectorized horizontal reduction returning wrong
Alexey Bataev via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 23 02:57:15 PST 2017
Author: abataev
Date: Thu Feb 23 04:57:15 2017
New Revision: 295956
URL: http://llvm.org/viewvc/llvm-project?rev=295956&view=rev
Log:
[SLP] Fix for PR32036: Vectorized horizontal reduction returning wrong
result
Summary:
If the same value is used several times as an extra value, SLP
vectorizer takes it into account only once instead of actual number of
using.
For example:
```
int val = 1;
for (int y = 0; y < 8; y++) {
for (int x = 0; x < 8; x++) {
val = val + input[y * 8 + x] + 3;
}
}
```
We have 2 extra rguments: `1` - initial value of horizontal reduction
and `3`, which is added 8*8 times to the reduction. Before the patch we
added `1` to the reduction value and added once `3`, though it must be
added 64 times.
Reviewers: mkuper, mzolotukhin
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D30262
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=295956&r1=295955&r2=295956&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp (original)
+++ llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp Thu Feb 23 04:57:15 2017
@@ -304,6 +304,7 @@ 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,
@@ -333,7 +334,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(MapVector<Value *, DebugLoc> &ExternallyUsedValues);
+ Value *vectorizeTree(ExtraValueToDebugLocsMap &ExternallyUsedValues);
/// \returns the cost incurred by unwanted spills and fills, caused by
/// holding live values over call sites.
@@ -352,7 +353,7 @@ public:
/// into account (anf updating it, if required) list of externally used
/// values stored in \p ExternallyUsedValues.
void buildTree(ArrayRef<Value *> Roots,
- MapVector<Value *, DebugLoc> &ExternallyUsedValues,
+ ExtraValueToDebugLocsMap &ExternallyUsedValues,
ArrayRef<Value *> UserIgnoreLst = None);
/// Clear the internal data structures that are created by 'buildTree'.
@@ -953,11 +954,11 @@ private:
void BoUpSLP::buildTree(ArrayRef<Value *> Roots,
ArrayRef<Value *> UserIgnoreLst) {
- MapVector<Value *, DebugLoc> ExternallyUsedValues;
+ ExtraValueToDebugLocsMap ExternallyUsedValues;
buildTree(Roots, ExternallyUsedValues, UserIgnoreLst);
}
void BoUpSLP::buildTree(ArrayRef<Value *> Roots,
- MapVector<Value *, DebugLoc> &ExternallyUsedValues,
+ ExtraValueToDebugLocsMap &ExternallyUsedValues,
ArrayRef<Value *> UserIgnoreLst) {
deleteTree();
UserIgnoreList = UserIgnoreLst;
@@ -2801,12 +2802,12 @@ Value *BoUpSLP::vectorizeTree(ArrayRef<V
}
Value *BoUpSLP::vectorizeTree() {
- MapVector<Value *, DebugLoc> ExternallyUsedValues;
+ ExtraValueToDebugLocsMap ExternallyUsedValues;
return vectorizeTree(ExternallyUsedValues);
}
Value *
-BoUpSLP::vectorizeTree(MapVector<Value *, DebugLoc> &ExternallyUsedValues) {
+BoUpSLP::vectorizeTree(ExtraValueToDebugLocsMap &ExternallyUsedValues) {
// All blocks must be scheduled before any instructions are inserted.
for (auto &BSIter : BlocksSchedules) {
@@ -2868,7 +2869,6 @@ BoUpSLP::vectorizeTree(MapVector<Value *
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(MapVector<Value *
Value *Ex = Builder.CreateExtractElement(Vec, Lane);
Ex = extend(ScalarRoot, Ex, Scalar->getType());
CSEBlocks.insert(cast<Instruction>(Scalar)->getParent());
- ExternallyUsedValues.erase(Scalar);
- ExternallyUsedValues[Ex] = DL;
+ std::swap(ExternallyUsedValues[Ex], ExternallyUsedValues[Scalar]);
+ assert(ExternallyUsedValues[Scalar].empty());
continue;
}
@@ -4439,9 +4439,11 @@ public:
Builder.setFastMathFlags(Unsafe);
unsigned i = 0;
- MapVector<Value *, DebugLoc> ExternallyUsedValues;
+ BoUpSLP::ExtraValueToDebugLocsMap ExternallyUsedValues;
+ // The same extra argument may be used several time, so log each attempt
+ // to use it.
for (auto &Pair : ExtraArgs)
- ExternallyUsedValues[Pair.second] = Pair.first->getDebugLoc();
+ ExternallyUsedValues[Pair.second].push_back(Pair.first->getDebugLoc());
while (i < NumReducedVals - ReduxWidth + 1 && ReduxWidth > 2) {
auto VL = makeArrayRef(&ReducedVals[i], ReduxWidth);
V.buildTree(VL, ExternallyUsedValues, ReductionOps);
@@ -4489,9 +4491,14 @@ public:
Builder.CreateBinOp(ReductionOpcode, VectorizedTree, I);
}
for (auto &Pair : ExternallyUsedValues) {
- Builder.SetCurrentDebugLocation(Pair.second);
- VectorizedTree = Builder.CreateBinOp(ReductionOpcode, VectorizedTree,
- Pair.first, "bin.extra");
+ 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");
+ }
}
// 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=295956&r1=295955&r2=295956&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/SLPVectorizer/X86/horizontal-list.ll (original)
+++ llvm/trunk/test/Transforms/SLPVectorizer/X86/horizontal-list.ll Thu Feb 23 04:57:15 2017
@@ -1473,9 +1473,10 @@ 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]], [[CONV]]
+; CHECK-NEXT: [[BIN_EXTRA6:%.*]] = fadd fast float [[BIN_EXTRA5]], 5.000000e+00
+; CHECK-NEXT: [[BIN_EXTRA7:%.*]] = fadd fast float [[BIN_EXTRA6]], [[CONV]]
; CHECK-NEXT: [[ADD4_6:%.*]] = fadd fast float undef, [[ADD4_5]]
-; CHECK-NEXT: ret float [[BIN_EXTRA6]]
+; CHECK-NEXT: ret float [[BIN_EXTRA7]]
;
; THRESHOLD-LABEL: @extra_args_same_several_times(
; THRESHOLD-NEXT: entry:
@@ -1510,9 +1511,10 @@ 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]], [[CONV]]
+; THRESHOLD-NEXT: [[BIN_EXTRA6:%.*]] = fadd fast float [[BIN_EXTRA5]], 5.000000e+00
+; THRESHOLD-NEXT: [[BIN_EXTRA7:%.*]] = fadd fast float [[BIN_EXTRA6]], [[CONV]]
; THRESHOLD-NEXT: [[ADD4_6:%.*]] = fadd fast float undef, [[ADD4_5]]
-; THRESHOLD-NEXT: ret float [[BIN_EXTRA6]]
+; THRESHOLD-NEXT: ret float [[BIN_EXTRA7]]
;
entry:
%mul = mul nsw i32 %b, %a
More information about the llvm-commits
mailing list