Skip to content

Commit

Permalink
Merging r321870, r321872, and r321994:
Browse files Browse the repository at this point in the history 
------------------------------------------------------------------------
r321870 | abataev | 2018-01-05 07:20:40 -0800 (Fri, 05 Jan 2018) | 1 line

[SLP] Update test checks, NFC.
------------------------------------------------------------------------

------------------------------------------------------------------------
r321872 | abataev | 2018-01-05 08:15:17 -0800 (Fri, 05 Jan 2018) | 1 line

[SLP] Update more test checks, NFC.
------------------------------------------------------------------------

------------------------------------------------------------------------
r321994 | abataev | 2018-01-08 06:43:06 -0800 (Mon, 08 Jan 2018) | 13 lines

[SLP] Fix PR35777: Incorrect handling of aggregate values.

Summary:
Fixes the bug with incorrect handling of InsertValue|InsertElement
instrucions in SLP vectorizer. Currently, we may use incorrect
ExtractElement instructions as the operands of the original
InsertValue|InsertElement instructions.

Reviewers: mkuper, hfinkel, RKSimon, spatel

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D41767
------------------------------------------------------------------------

llvm-svn: 322675
  • Loading branch information
@zmodem
zmodem committed Jan 17, 2018
1 parent cf9e4fb commit 0462903
Show file tree
Hide file tree
Showing 6 changed files with 348 additions and 197 deletions.
7 changes: 1 addition & 6 deletions llvm/include/llvm/Transforms/Vectorize/SLPVectorizer.h
View file
Expand Up @@ -95,14 +95,9 @@ struct SLPVectorizerPass : public PassInfoMixin<SLPVectorizerPass> {
bool tryToVectorizePair(Value *A, Value *B, slpvectorizer::BoUpSLP &R);

/// \brief Try to vectorize a list of operands.
/// \@param BuildVector A list of users to ignore for the purpose of
/// scheduling and cost estimation when NeedExtraction
/// is false.
/// \returns true if a value was vectorized.
bool tryToVectorizeList(ArrayRef<Value *> VL, slpvectorizer::BoUpSLP &R,
ArrayRef<Value *> BuildVector = None,
bool AllowReorder = false,
bool NeedExtraction = false);
bool AllowReorder = false);

/// \brief Try to vectorize a chain that may start at the operands of \p I.
bool tryToVectorize(Instruction *I, slpvectorizer::BoUpSLP &R);
Expand Down
60 changes: 10 additions & 50 deletions llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
View file
Expand Up @@ -4416,13 +4416,11 @@ bool SLPVectorizerPass::tryToVectorizePair(Value *A, Value *B, BoUpSLP &R) {
if (!A || !B)
return false;
Value *VL[] = { A, B };
return tryToVectorizeList(VL, R, None, true);
return tryToVectorizeList(VL, R, true);
}

bool SLPVectorizerPass::tryToVectorizeList(ArrayRef<Value *> VL, BoUpSLP &R,
ArrayRef<Value *> BuildVector,
bool AllowReorder,
bool NeedExtraction) {
bool AllowReorder) {
if (VL.size() < 2)
return false;

Expand Down Expand Up @@ -4516,20 +4514,14 @@ bool SLPVectorizerPass::tryToVectorizeList(ArrayRef<Value *> VL, BoUpSLP &R,
<< "\n");
ArrayRef<Value *> Ops = VL.slice(I, OpsWidth);

ArrayRef<Value *> EmptyArray;
ArrayRef<Value *> BuildVectorSlice;
if (!BuildVector.empty())
BuildVectorSlice = BuildVector.slice(I, OpsWidth);

R.buildTree(Ops, NeedExtraction ? EmptyArray : BuildVectorSlice);
R.buildTree(Ops);
// TODO: check if we can allow reordering for more cases.
if (AllowReorder && R.shouldReorder()) {
// Conceptually, there is nothing actually preventing us from trying to
// reorder a larger list. In fact, we do exactly this when vectorizing
// reductions. However, at this point, we only expect to get here when
// there are exactly two operations.
assert(Ops.size() == 2);
assert(BuildVectorSlice.empty());
Value *ReorderedOps[] = {Ops[1], Ops[0]};
R.buildTree(ReorderedOps, None);
}
Expand All @@ -4549,31 +4541,7 @@ bool SLPVectorizerPass::tryToVectorizeList(ArrayRef<Value *> VL, BoUpSLP &R,
<< " and with tree size "
<< ore::NV("TreeSize", R.getTreeSize()));

Value *VectorizedRoot = 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()) {
// The insert point is the last build vector instruction. The
// vectorized root will precede it. This guarantees that we get an
// instruction. The vectorized tree could have been constant folded.
Instruction *InsertAfter = cast<Instruction>(BuildVectorSlice.back());
unsigned VecIdx = 0;
for (auto &V : BuildVectorSlice) {
IRBuilder<NoFolder> Builder(InsertAfter->getParent(),
++BasicBlock::iterator(InsertAfter));
Instruction *I = cast<Instruction>(V);
assert(isa<InsertElementInst>(I) || isa<InsertValueInst>(I));
Instruction *Extract =
cast<Instruction>(Builder.CreateExtractElement(
VectorizedRoot, Builder.getInt32(VecIdx++)));
I->setOperand(1, Extract);
I->moveAfter(Extract);
InsertAfter = I;
}
}
R.vectorizeTree();
// Move to the next bundle.
I += VF - 1;
NextInst = I + 1;
Expand Down Expand Up @@ -5494,11 +5462,9 @@ class HorizontalReduction {
///
/// Returns true if it matches
static bool findBuildVector(InsertElementInst *LastInsertElem,
SmallVectorImpl<Value *> &BuildVector,
SmallVectorImpl<Value *> &BuildVectorOpds) {
Value *V = nullptr;
do {
BuildVector.push_back(LastInsertElem);
BuildVectorOpds.push_back(LastInsertElem->getOperand(1));
V = LastInsertElem->getOperand(0);
if (isa<UndefValue>(V))
Expand All @@ -5507,7 +5473,6 @@ static bool findBuildVector(InsertElementInst *LastInsertElem,
if (!LastInsertElem || !LastInsertElem->hasOneUse())
return false;
} while (true);
std::reverse(BuildVector.begin(), BuildVector.end());
std::reverse(BuildVectorOpds.begin(), BuildVectorOpds.end());
return true;
}
Expand All @@ -5516,11 +5481,9 @@ static bool findBuildVector(InsertElementInst *LastInsertElem,
///
/// \return true if it matches.
static bool findBuildAggregate(InsertValueInst *IV,
SmallVectorImpl<Value *> &BuildVector,
SmallVectorImpl<Value *> &BuildVectorOpds) {
Value *V;
do {
BuildVector.push_back(IV);
BuildVectorOpds.push_back(IV->getInsertedValueOperand());
V = IV->getAggregateOperand();
if (isa<UndefValue>(V))
Expand All @@ -5529,7 +5492,6 @@ static bool findBuildAggregate(InsertValueInst *IV,
if (!IV || !IV->hasOneUse())
return false;
} while (true);
std::reverse(BuildVector.begin(), BuildVector.end());
std::reverse(BuildVectorOpds.begin(), BuildVectorOpds.end());
return true;
}
Expand Down Expand Up @@ -5705,27 +5667,25 @@ bool SLPVectorizerPass::vectorizeInsertValueInst(InsertValueInst *IVI,
if (!R.canMapToVector(IVI->getType(), DL))
return false;

SmallVector<Value *, 16> BuildVector;
SmallVector<Value *, 16> BuildVectorOpds;
if (!findBuildAggregate(IVI, BuildVector, BuildVectorOpds))
if (!findBuildAggregate(IVI, BuildVectorOpds))
return false;

DEBUG(dbgs() << "SLP: array mappable to vector: " << *IVI << "\n");
// Aggregate value is unlikely to be processed in vector register, we need to
// extract scalars into scalar registers, so NeedExtraction is set true.
return tryToVectorizeList(BuildVectorOpds, R, BuildVector, false, true);
return tryToVectorizeList(BuildVectorOpds, R);
}

bool SLPVectorizerPass::vectorizeInsertElementInst(InsertElementInst *IEI,
BasicBlock *BB, BoUpSLP &R) {
SmallVector<Value *, 16> BuildVector;
SmallVector<Value *, 16> BuildVectorOpds;
if (!findBuildVector(IEI, BuildVector, BuildVectorOpds))
if (!findBuildVector(IEI, BuildVectorOpds))
return false;

// Vectorize starting with the build vector operands ignoring the BuildVector
// instructions for the purpose of scheduling and user extraction.
return tryToVectorizeList(BuildVectorOpds, R, BuildVector);
return tryToVectorizeList(BuildVectorOpds, R);
}

bool SLPVectorizerPass::vectorizeCmpInst(CmpInst *CI, BasicBlock *BB,
Expand Down Expand Up @@ -5803,8 +5763,8 @@ bool SLPVectorizerPass::vectorizeChainsInBlock(BasicBlock *BB, BoUpSLP &R) {
// is done when there are exactly two elements since tryToVectorizeList
// asserts that there are only two values when AllowReorder is true.
bool AllowReorder = NumElts == 2;
if (NumElts > 1 && tryToVectorizeList(makeArrayRef(IncIt, NumElts), R,
None, AllowReorder)) {
if (NumElts > 1 &&
tryToVectorizeList(makeArrayRef(IncIt, NumElts), R, AllowReorder)) {
// Success start over because instructions might have been changed.
HaveVectorizedPhiNodes = true;
Changed = true;
Expand Down
48 changes: 48 additions & 0 deletions llvm/test/Transforms/SLPVectorizer/X86/PR35777.ll
View file
@@ -0,0 +1,48 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt < %s -verify -slp-vectorizer -o - -S -mtriple=x86_64-apple-macosx10.13.0 | FileCheck %s

@global = local_unnamed_addr global [6 x double] zeroinitializer, align 16

define { i64, i64 } @patatino(double %arg) {
; CHECK-LABEL: @patatino(
; CHECK-NEXT: bb:
; CHECK-NEXT: [[TMP0:%.*]] = load <2 x double>, <2 x double>* bitcast ([6 x double]* @global to <2 x double>*), align 16
; CHECK-NEXT: [[TMP1:%.*]] = load <2 x double>, <2 x double>* bitcast (double* getelementptr inbounds ([6 x double], [6 x double]* @global, i64 0, i64 2) to <2 x double>*), align 16
; CHECK-NEXT: [[TMP2:%.*]] = insertelement <2 x double> undef, double [[ARG:%.*]], i32 0
; CHECK-NEXT: [[TMP3:%.*]] = insertelement <2 x double> [[TMP2]], double [[ARG]], i32 1
; CHECK-NEXT: [[TMP4:%.*]] = fmul <2 x double> [[TMP3]], [[TMP1]]
; CHECK-NEXT: [[TMP5:%.*]] = fadd <2 x double> [[TMP0]], [[TMP4]]
; CHECK-NEXT: [[TMP6:%.*]] = load <2 x double>, <2 x double>* bitcast (double* getelementptr inbounds ([6 x double], [6 x double]* @global, i64 0, i64 4) to <2 x double>*), align 16
; CHECK-NEXT: [[TMP7:%.*]] = fadd <2 x double> [[TMP6]], [[TMP5]]
; CHECK-NEXT: [[TMP8:%.*]] = fptosi <2 x double> [[TMP7]] to <2 x i32>
; CHECK-NEXT: [[TMP9:%.*]] = sext <2 x i32> [[TMP8]] to <2 x i64>
; CHECK-NEXT: [[TMP10:%.*]] = trunc <2 x i64> [[TMP9]] to <2 x i32>
; CHECK-NEXT: [[TMP11:%.*]] = extractelement <2 x i32> [[TMP10]], i32 0
; CHECK-NEXT: [[TMP12:%.*]] = sext i32 [[TMP11]] to i64
; CHECK-NEXT: [[TMP16:%.*]] = insertvalue { i64, i64 } undef, i64 [[TMP12]], 0
; CHECK-NEXT: [[TMP13:%.*]] = extractelement <2 x i32> [[TMP10]], i32 1
; CHECK-NEXT: [[TMP14:%.*]] = sext i32 [[TMP13]] to i64
; CHECK-NEXT: [[TMP17:%.*]] = insertvalue { i64, i64 } [[TMP16]], i64 [[TMP14]], 1
; CHECK-NEXT: ret { i64, i64 } [[TMP17]]
;
bb:
%tmp = load double, double* getelementptr inbounds ([6 x double], [6 x double]* @global, i64 0, i64 0), align 16
%tmp1 = load double, double* getelementptr inbounds ([6 x double], [6 x double]* @global, i64 0, i64 2), align 16
%tmp2 = fmul double %tmp1, %arg
%tmp3 = fadd double %tmp, %tmp2
%tmp4 = load double, double* getelementptr inbounds ([6 x double], [6 x double]* @global, i64 0, i64 4), align 16
%tmp5 = fadd double %tmp4, %tmp3
%tmp6 = fptosi double %tmp5 to i32
%tmp7 = sext i32 %tmp6 to i64
%tmp8 = load double, double* getelementptr inbounds ([6 x double], [6 x double]* @global, i64 0, i64 1), align 8
%tmp9 = load double, double* getelementptr inbounds ([6 x double], [6 x double]* @global, i64 0, i64 3), align 8
%tmp10 = fmul double %tmp9, %arg
%tmp11 = fadd double %tmp8, %tmp10
%tmp12 = load double, double* getelementptr inbounds ([6 x double], [6 x double]* @global, i64 0, i64 5), align 8
%tmp13 = fadd double %tmp12, %tmp11
%tmp14 = fptosi double %tmp13 to i32
%tmp15 = sext i32 %tmp14 to i64
%tmp16 = insertvalue { i64, i64 } undef, i64 %tmp7, 0
%tmp17 = insertvalue { i64, i64 } %tmp16, i64 %tmp15, 1
ret { i64, i64 } %tmp17
}

0 comments on commit 0462903

Please sign in to comment.