From 507b9242ed3cbac13a1c4c58fe28188e1a0d6fa6 Mon Sep 17 00:00:00 2001 From: Nadav Rotem <nrotem@apple.com> Date: Sun, 12 May 2013 22:58:45 +0000 Subject: [PATCH] SLPVectorizer: Fix a bug in the code that generates extracts for values with multiple users. The external user does not have to be in lane #0. We have to save the lane for each scalar so that we know which vector lane to extract. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@181674 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Vectorize/VecUtils.cpp | 34 ++++++++++++++++---- test/Transforms/SLPVectorizer/X86/diamond.ll | 32 ++++++++++++++++-- 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/lib/Transforms/Vectorize/VecUtils.cpp b/lib/Transforms/Vectorize/VecUtils.cpp index 6f36c938fa5..1362b784f06 100644 --- a/lib/Transforms/Vectorize/VecUtils.cpp +++ b/lib/Transforms/Vectorize/VecUtils.cpp @@ -282,6 +282,7 @@ int BoUpSLP::getTreeCost(ArrayRef<Value *> VL) { DEBUG(dbgs()<<"SLP: Adding to MustExtract " "because of a safe out of tree usage.\n"); MustExtract.insert(*it); + continue; } if (Lane == -1) Lane = LaneMap[*I]; if (Lane != LaneMap[*I]) { @@ -610,6 +611,9 @@ Value *BoUpSLP::Scalarize(ArrayRef<Value *> VL, VectorType *Ty) { GatherInstructions.push_back(Vec); } + for (unsigned i = 0; i < Ty->getNumElements(); ++i) + VectorizedValues[VL[i]] = Vec; + return Vec; } @@ -617,6 +621,7 @@ Value *BoUpSLP::vectorizeTree(ArrayRef<Value *> VL, int VF) { Value *V = vectorizeTree_rec(VL, VF); Instruction *LastInstr = GetLastInstr(VL, VL.size()); + int LastInstrIdx = InstrIdx[LastInstr]; IRBuilder<> Builder(LastInstr); for (ValueSet::iterator it = MustExtract.begin(), e = MustExtract.end(); it != e; ++it) { @@ -625,7 +630,15 @@ Value *BoUpSLP::vectorizeTree(ArrayRef<Value *> VL, int VF) { assert(LaneMap.count(I) && "Unable to find the lane for the external use"); Value *Idx = Builder.getInt32(LaneMap[I]); Value *Extract = Builder.CreateExtractElement(Vec, Idx); - I->replaceAllUsesWith(Extract); + bool Replaced = false; + for (Value::use_iterator U = I->use_begin(), UE = U->use_end(); U != UE; + ++U) { + Instruction *UI = cast<Instruction>(*U); + if (UI->getParent() != I->getParent() || InstrIdx[UI] > LastInstrIdx) + UI->replaceUsesOfWith(I ,Extract); + Replaced = true; + } + assert(Replaced && "Must replace at least one outside user"); } // We moved some instructions around. We have to number them again @@ -691,7 +704,10 @@ Value *BoUpSLP::vectorizeTree_rec(ArrayRef<Value *> VL, int VF) { IRBuilder<> Builder(GetLastInstr(VL, VF)); CastInst *CI = dyn_cast<CastInst>(VL0); Value *V = Builder.CreateCast(CI->getOpcode(), InVec, VecTy); - VectorizedValues[VL0] = V; + + for (int i = 0; i < VF; ++i) + VectorizedValues[VL[i]] = V; + return V; } case Instruction::Add: @@ -723,7 +739,10 @@ Value *BoUpSLP::vectorizeTree_rec(ArrayRef<Value *> VL, int VF) { IRBuilder<> Builder(GetLastInstr(VL, VF)); BinaryOperator *BinOp = cast<BinaryOperator>(VL0); Value *V = Builder.CreateBinOp(BinOp->getOpcode(), RHS,LHS); - VectorizedValues[VL0] = V; + + for (int i = 0; i < VF; ++i) + VectorizedValues[VL[i]] = V; + return V; } case Instruction::Load: { @@ -740,7 +759,10 @@ Value *BoUpSLP::vectorizeTree_rec(ArrayRef<Value *> VL, int VF) { VecTy->getPointerTo()); LI = Builder.CreateLoad(VecPtr); LI->setAlignment(Alignment); - VectorizedValues[VL0] = LI; + + for (int i = 0; i < VF; ++i) + VectorizedValues[VL[i]] = LI; + return LI; } case Instruction::Store: { @@ -763,9 +785,7 @@ Value *BoUpSLP::vectorizeTree_rec(ArrayRef<Value *> VL, int VF) { return 0; } default: - Value *S = Scalarize(VL, VecTy); - VectorizedValues[VL0] = S; - return S; + return Scalarize(VL, VecTy); } } diff --git a/test/Transforms/SLPVectorizer/X86/diamond.ll b/test/Transforms/SLPVectorizer/X86/diamond.ll index 49c8712d202..8959b0d9eec 100644 --- a/test/Transforms/SLPVectorizer/X86/diamond.ll +++ b/test/Transforms/SLPVectorizer/X86/diamond.ll @@ -1,4 +1,4 @@ -; RUN: opt < %s -basicaa -slp-vectorizer -dce -S -mtriple=x86_64-apple-macosx10.8.0 -mcpu=corei7-avx | FileCheck %s +; RUN: opt < %s -basicaa -slp-vectorizer -S -mtriple=x86_64-apple-macosx10.8.0 -mcpu=corei7-avx | FileCheck %s target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128" target triple = "x86_64-apple-macosx10.8.0" @@ -74,6 +74,34 @@ entry: %add20 = mul i32 %3, %mul238 %arrayidx21 = getelementptr inbounds i32* %B, i64 3 store i32 %add20, i32* %arrayidx21, align 4 - ret i32 %0 ;<--------- This value has multiple users and can't be vectorized. + ret i32 %0 ;<--------- This value has multiple users } +; In this example we have an external user that is not the first element in the vector. +; CHECK: @extr_user1 +; CHECK: store <4 x i32> +; CHECK-NEXT: extractelement <4 x i32> +; CHECK: ret +define i32 @extr_user1(i32* noalias nocapture %B, i32* noalias nocapture %A, i32 %n, i32 %m) { +entry: + %0 = load i32* %A, align 4 + %mul238 = add i32 %m, %n + %add = mul i32 %0, %mul238 + store i32 %add, i32* %B, align 4 + %arrayidx4 = getelementptr inbounds i32* %A, i64 1 + %1 = load i32* %arrayidx4, align 4 + %add8 = mul i32 %1, %mul238 + %arrayidx9 = getelementptr inbounds i32* %B, i64 1 + store i32 %add8, i32* %arrayidx9, align 4 + %arrayidx10 = getelementptr inbounds i32* %A, i64 2 + %2 = load i32* %arrayidx10, align 4 + %add14 = mul i32 %2, %mul238 + %arrayidx15 = getelementptr inbounds i32* %B, i64 2 + store i32 %add14, i32* %arrayidx15, align 4 + %arrayidx16 = getelementptr inbounds i32* %A, i64 3 + %3 = load i32* %arrayidx16, align 4 + %add20 = mul i32 %3, %mul238 + %arrayidx21 = getelementptr inbounds i32* %B, i64 3 + store i32 %add20, i32* %arrayidx21, align 4 + ret i32 %1 ;<--------- This value has multiple users +}