[llvm] r276938 - [LSV] Use Instruction*s rather than Value*s where possible.
Justin Lebar via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 27 16:06:00 PDT 2016
Author: jlebar
Date: Wed Jul 27 18:06:00 2016
New Revision: 276938
URL: http://llvm.org/viewvc/llvm-project?rev=276938&view=rev
Log:
[LSV] Use Instruction*s rather than Value*s where possible.
Summary:
Given the crash in D22878, this patch converts the load/store vectorizer
to use explicit Instruction*s wherever possible. This is an overall
simplification and should be an improvement in safety, as we have fewer
naked cast<>s, and now where we use Value*, we really mean something
different from Instruction*.
This patch also gets rid of some cast<>s around Value*s returned by
Builder. Given that Builder constant-folds everything, we can't assume
much about what we get out of it.
One downside of this patch is that we have to copy our chain before
calling propagateMetadata. But I don't think this is a big deal, as our
chains are very small (usually 2 or 4 elems).
Reviewers: asbirlea
Subscribers: mzolotukhin, llvm-commits, arsenm
Differential Revision: https://reviews.llvm.org/D22887
Modified:
llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
Modified: llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp?rev=276938&r1=276937&r2=276938&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp (original)
+++ llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp Wed Jul 27 18:06:00 2016
@@ -43,8 +43,8 @@ namespace {
// TODO: Remove this
static const unsigned TargetBaseAlign = 4;
-typedef SmallVector<Value *, 8> ValueList;
-typedef MapVector<Value *, ValueList> ValueListMap;
+typedef SmallVector<Instruction *, 8> InstrList;
+typedef MapVector<Value *, InstrList> InstrListMap;
class Vectorizer {
Function &F;
@@ -92,17 +92,17 @@ private:
/// Returns the first and the last instructions in Chain.
std::pair<BasicBlock::iterator, BasicBlock::iterator>
- getBoundaryInstrs(ArrayRef<Value *> Chain);
+ getBoundaryInstrs(ArrayRef<Instruction *> Chain);
/// Erases the original instructions after vectorizing.
- void eraseInstructions(ArrayRef<Value *> Chain);
+ void eraseInstructions(ArrayRef<Instruction *> Chain);
/// "Legalize" the vector type that would be produced by combining \p
/// ElementSizeBits elements in \p Chain. Break into two pieces such that the
/// total size of each piece is 1, 2 or a multiple of 4 bytes. \p Chain is
/// expected to have more than 4 elements.
- std::pair<ArrayRef<Value *>, ArrayRef<Value *>>
- splitOddVectorElts(ArrayRef<Value *> Chain, unsigned ElementSizeBits);
+ std::pair<ArrayRef<Instruction *>, ArrayRef<Instruction *>>
+ splitOddVectorElts(ArrayRef<Instruction *> Chain, unsigned ElementSizeBits);
/// Finds the largest prefix of Chain that's vectorizable, checking for
/// intervening instructions which may affect the memory accessed by the
@@ -110,25 +110,27 @@ private:
///
/// The elements of \p Chain must be all loads or all stores and must be in
/// address order.
- ArrayRef<Value *> getVectorizablePrefix(ArrayRef<Value *> Chain);
+ ArrayRef<Instruction *> getVectorizablePrefix(ArrayRef<Instruction *> Chain);
/// Collects load and store instructions to vectorize.
- std::pair<ValueListMap, ValueListMap> collectInstructions(BasicBlock *BB);
+ std::pair<InstrListMap, InstrListMap> collectInstructions(BasicBlock *BB);
- /// Processes the collected instructions, the \p Map. The elements of \p Map
+ /// Processes the collected instructions, the \p Map. The values of \p Map
/// should be all loads or all stores.
- bool vectorizeChains(ValueListMap &Map);
+ bool vectorizeChains(InstrListMap &Map);
/// Finds the load/stores to consecutive memory addresses and vectorizes them.
- bool vectorizeInstructions(ArrayRef<Value *> Instrs);
+ bool vectorizeInstructions(ArrayRef<Instruction *> Instrs);
/// Vectorizes the load instructions in Chain.
- bool vectorizeLoadChain(ArrayRef<Value *> Chain,
- SmallPtrSet<Value *, 16> *InstructionsProcessed);
+ bool
+ vectorizeLoadChain(ArrayRef<Instruction *> Chain,
+ SmallPtrSet<Instruction *, 16> *InstructionsProcessed);
/// Vectorizes the store instructions in Chain.
- bool vectorizeStoreChain(ArrayRef<Value *> Chain,
- SmallPtrSet<Value *, 16> *InstructionsProcessed);
+ bool
+ vectorizeStoreChain(ArrayRef<Instruction *> Chain,
+ SmallPtrSet<Instruction *, 16> *InstructionsProcessed);
/// Check if this load/store access is misaligned accesses
bool accessIsMisaligned(unsigned SzInBytes, unsigned AddressSpace,
@@ -175,6 +177,13 @@ Pass *llvm::createLoadStoreVectorizerPas
return new LoadStoreVectorizer();
}
+// The real propagateMetadata expects a SmallVector<Value*>, but we deal in
+// vectors of Instructions.
+static void propagateMetadata(Instruction *I, ArrayRef<Instruction *> IL) {
+ SmallVector<Value *, 8> VL(IL.begin(), IL.end());
+ propagateMetadata(I, VL);
+}
+
bool LoadStoreVectorizer::runOnFunction(Function &F) {
// Don't vectorize when the attribute NoImplicitFloat is used.
if (skipFunction(F) || F.hasFnAttribute(Attribute::NoImplicitFloat))
@@ -196,7 +205,7 @@ bool Vectorizer::run() {
// Scan the blocks in the function in post order.
for (BasicBlock *BB : post_order(&F)) {
- ValueListMap LoadRefs, StoreRefs;
+ InstrListMap LoadRefs, StoreRefs;
std::tie(LoadRefs, StoreRefs) = collectInstructions(BB);
Changed |= vectorizeChains(LoadRefs);
Changed |= vectorizeChains(StoreRefs);
@@ -371,8 +380,8 @@ void Vectorizer::reorder(Instruction *I)
}
std::pair<BasicBlock::iterator, BasicBlock::iterator>
-Vectorizer::getBoundaryInstrs(ArrayRef<Value *> Chain) {
- Instruction *C0 = cast<Instruction>(Chain[0]);
+Vectorizer::getBoundaryInstrs(ArrayRef<Instruction *> Chain) {
+ Instruction *C0 = Chain[0];
BasicBlock::iterator FirstInstr = C0->getIterator();
BasicBlock::iterator LastInstr = C0->getIterator();
@@ -396,26 +405,24 @@ Vectorizer::getBoundaryInstrs(ArrayRef<V
return std::make_pair(FirstInstr, ++LastInstr);
}
-void Vectorizer::eraseInstructions(ArrayRef<Value *> Chain) {
+void Vectorizer::eraseInstructions(ArrayRef<Instruction *> Chain) {
SmallVector<Instruction *, 16> Instrs;
- for (Value *V : Chain) {
- Value *PtrOperand = getPointerOperand(V);
+ for (Instruction *I : Chain) {
+ Value *PtrOperand = getPointerOperand(I);
assert(PtrOperand && "Instruction must have a pointer operand.");
- Instrs.push_back(cast<Instruction>(V));
+ Instrs.push_back(I);
if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(PtrOperand))
Instrs.push_back(GEP);
}
// Erase instructions.
- for (Value *V : Instrs) {
- Instruction *Instr = cast<Instruction>(V);
- if (Instr->use_empty())
- Instr->eraseFromParent();
- }
+ for (Instruction *I : Instrs)
+ if (I->use_empty())
+ I->eraseFromParent();
}
-std::pair<ArrayRef<Value *>, ArrayRef<Value *>>
-Vectorizer::splitOddVectorElts(ArrayRef<Value *> Chain,
+std::pair<ArrayRef<Instruction *>, ArrayRef<Instruction *>>
+Vectorizer::splitOddVectorElts(ArrayRef<Instruction *> Chain,
unsigned ElementSizeBits) {
unsigned ElemSizeInBytes = ElementSizeBits / 8;
unsigned SizeInBytes = ElemSizeInBytes * Chain.size();
@@ -424,19 +431,20 @@ Vectorizer::splitOddVectorElts(ArrayRef<
return std::make_pair(Chain.slice(0, NumLeft), Chain.slice(NumLeft));
}
-ArrayRef<Value *> Vectorizer::getVectorizablePrefix(ArrayRef<Value *> Chain) {
+ArrayRef<Instruction *>
+Vectorizer::getVectorizablePrefix(ArrayRef<Instruction *> Chain) {
// These are in BB order, unlike Chain, which is in address order.
- SmallVector<std::pair<Value *, unsigned>, 16> MemoryInstrs;
- SmallVector<std::pair<Value *, unsigned>, 16> ChainInstrs;
+ SmallVector<std::pair<Instruction *, unsigned>, 16> MemoryInstrs;
+ SmallVector<std::pair<Instruction *, unsigned>, 16> ChainInstrs;
bool IsLoadChain = isa<LoadInst>(Chain[0]);
DEBUG({
- for (Value *V : Chain) {
+ for (Instruction *I : Chain) {
if (IsLoadChain)
- assert(isa<LoadInst>(V) &&
+ assert(isa<LoadInst>(I) &&
"All elements of Chain must be loads, or all must be stores.");
else
- assert(isa<StoreInst>(V) &&
+ assert(isa<StoreInst>(I) &&
"All elements of Chain must be loads, or all must be stores.");
}
});
@@ -463,11 +471,11 @@ ArrayRef<Value *> Vectorizer::getVectori
unsigned ChainInstrIdx, ChainInstrsLen;
for (ChainInstrIdx = 0, ChainInstrsLen = ChainInstrs.size();
ChainInstrIdx < ChainInstrsLen; ++ChainInstrIdx) {
- Value *ChainInstr = ChainInstrs[ChainInstrIdx].first;
+ Instruction *ChainInstr = ChainInstrs[ChainInstrIdx].first;
unsigned ChainInstrLoc = ChainInstrs[ChainInstrIdx].second;
bool AliasFound = false;
for (auto EntryMem : MemoryInstrs) {
- Value *MemInstr = EntryMem.first;
+ Instruction *MemInstr = EntryMem.first;
unsigned MemInstrLoc = EntryMem.second;
if (isa<LoadInst>(MemInstr) && isa<LoadInst>(ChainInstr))
continue;
@@ -485,20 +493,16 @@ ArrayRef<Value *> Vectorizer::getVectori
ChainInstrLoc > MemInstrLoc)
continue;
- Instruction *M0 = cast<Instruction>(MemInstr);
- Instruction *M1 = cast<Instruction>(ChainInstr);
-
- if (!AA.isNoAlias(MemoryLocation::get(M0), MemoryLocation::get(M1))) {
+ if (!AA.isNoAlias(MemoryLocation::get(MemInstr),
+ MemoryLocation::get(ChainInstr))) {
DEBUG({
- Value *Ptr0 = getPointerOperand(M0);
- Value *Ptr1 = getPointerOperand(M1);
dbgs() << "LSV: Found alias:\n"
" Aliasing instruction and pointer:\n"
<< " " << *MemInstr << '\n'
- << " " << *Ptr0 << '\n'
+ << " " << *getPointerOperand(MemInstr) << '\n'
<< " Aliased instruction and pointer:\n"
<< " " << *ChainInstr << '\n'
- << " " << *Ptr1 << '\n';
+ << " " << *getPointerOperand(ChainInstr) << '\n';
});
AliasFound = true;
break;
@@ -516,18 +520,20 @@ ArrayRef<Value *> Vectorizer::getVectori
makeArrayRef(ChainInstrs.data(), ChainInstrIdx);
unsigned ChainIdx, ChainLen;
for (ChainIdx = 0, ChainLen = Chain.size(); ChainIdx < ChainLen; ++ChainIdx) {
- Value *V = Chain[ChainIdx];
+ Instruction *I = Chain[ChainIdx];
if (!any_of(VectorizableChainInstrs,
- [V](std::pair<Value *, unsigned> CI) { return V == CI.first; }))
+ [I](std::pair<Instruction *, unsigned> CI) {
+ return I == CI.first;
+ }))
break;
}
return Chain.slice(0, ChainIdx);
}
-std::pair<ValueListMap, ValueListMap>
+std::pair<InstrListMap, InstrListMap>
Vectorizer::collectInstructions(BasicBlock *BB) {
- ValueListMap LoadRefs;
- ValueListMap StoreRefs;
+ InstrListMap LoadRefs;
+ InstrListMap StoreRefs;
for (Instruction &I : *BB) {
if (!I.mayReadOrWriteMemory())
@@ -557,9 +563,8 @@ Vectorizer::collectInstructions(BasicBlo
// Make sure all the users of a vector are constant-index extracts.
if (isa<VectorType>(Ty) && !all_of(LI->users(), [LI](const User *U) {
- const Instruction *UI = cast<Instruction>(U);
- return isa<ExtractElementInst>(UI) &&
- isa<ConstantInt>(UI->getOperand(1));
+ const ExtractElementInst *EEI = dyn_cast<ExtractElementInst>(U);
+ return EEI && isa<ConstantInt>(EEI->getOperand(1));
}))
continue;
@@ -590,9 +595,8 @@ Vectorizer::collectInstructions(BasicBlo
continue;
if (isa<VectorType>(Ty) && !all_of(SI->users(), [SI](const User *U) {
- const Instruction *UI = cast<Instruction>(U);
- return isa<ExtractElementInst>(UI) &&
- isa<ConstantInt>(UI->getOperand(1));
+ const ExtractElementInst *EEI = dyn_cast<ExtractElementInst>(U);
+ return EEI && isa<ConstantInt>(EEI->getOperand(1));
}))
continue;
@@ -605,10 +609,10 @@ Vectorizer::collectInstructions(BasicBlo
return {LoadRefs, StoreRefs};
}
-bool Vectorizer::vectorizeChains(ValueListMap &Map) {
+bool Vectorizer::vectorizeChains(InstrListMap &Map) {
bool Changed = false;
- for (const std::pair<Value *, ValueList> &Chain : Map) {
+ for (const std::pair<Value *, InstrList> &Chain : Map) {
unsigned Size = Chain.second.size();
if (Size < 2)
continue;
@@ -618,7 +622,7 @@ bool Vectorizer::vectorizeChains(ValueLi
// Process the stores in chunks of 64.
for (unsigned CI = 0, CE = Size; CI < CE; CI += 64) {
unsigned Len = std::min<unsigned>(CE - CI, 64);
- ArrayRef<Value *> Chunk(&Chain.second[CI], Len);
+ ArrayRef<Instruction *> Chunk(&Chain.second[CI], Len);
Changed |= vectorizeInstructions(Chunk);
}
}
@@ -626,7 +630,7 @@ bool Vectorizer::vectorizeChains(ValueLi
return Changed;
}
-bool Vectorizer::vectorizeInstructions(ArrayRef<Value *> Instrs) {
+bool Vectorizer::vectorizeInstructions(ArrayRef<Instruction *> Instrs) {
DEBUG(dbgs() << "LSV: Vectorizing " << Instrs.size() << " instructions.\n");
SmallSetVector<int, 16> Heads, Tails;
int ConsecutiveChain[64];
@@ -655,7 +659,7 @@ bool Vectorizer::vectorizeInstructions(A
}
bool Changed = false;
- SmallPtrSet<Value *, 16> InstructionsProcessed;
+ SmallPtrSet<Instruction *, 16> InstructionsProcessed;
for (int Head : Heads) {
if (InstructionsProcessed.count(Instrs[Head]))
@@ -672,7 +676,7 @@ bool Vectorizer::vectorizeInstructions(A
// We found an instr that starts a chain. Now follow the chain and try to
// vectorize it.
- SmallVector<Value *, 16> Operands;
+ SmallVector<Instruction *, 16> Operands;
int I = Head;
while (I != -1 && (Tails.count(I) || Heads.count(I))) {
if (InstructionsProcessed.count(Instrs[I]))
@@ -695,13 +699,14 @@ bool Vectorizer::vectorizeInstructions(A
}
bool Vectorizer::vectorizeStoreChain(
- ArrayRef<Value *> Chain, SmallPtrSet<Value *, 16> *InstructionsProcessed) {
+ ArrayRef<Instruction *> Chain,
+ SmallPtrSet<Instruction *, 16> *InstructionsProcessed) {
StoreInst *S0 = cast<StoreInst>(Chain[0]);
// If the vector has an int element, default to int for the whole load.
Type *StoreTy;
- for (const auto &V : Chain) {
- StoreTy = cast<StoreInst>(V)->getValueOperand()->getType();
+ for (Instruction *I : Chain) {
+ StoreTy = cast<StoreInst>(I)->getValueOperand()->getType();
if (StoreTy->isIntOrIntVectorTy())
break;
@@ -723,7 +728,7 @@ bool Vectorizer::vectorizeStoreChain(
return false;
}
- ArrayRef<Value *> NewChain = getVectorizablePrefix(Chain);
+ ArrayRef<Instruction *> NewChain = getVectorizablePrefix(Chain);
if (NewChain.empty()) {
// No vectorization possible.
InstructionsProcessed->insert(Chain.begin(), Chain.end());
@@ -773,8 +778,8 @@ bool Vectorizer::vectorizeStoreChain(
DEBUG({
dbgs() << "LSV: Stores to vectorize:\n";
- for (Value *V : Chain)
- dbgs() << " " << *V << "\n";
+ for (Instruction *I : Chain)
+ dbgs() << " " << *I << "\n";
});
// We won't try again to vectorize the elements of the chain, regardless of
@@ -836,9 +841,11 @@ bool Vectorizer::vectorizeStoreChain(
}
}
- Value *Bitcast =
- Builder.CreateBitCast(S0->getPointerOperand(), VecTy->getPointerTo(AS));
- StoreInst *SI = cast<StoreInst>(Builder.CreateStore(Vec, Bitcast));
+ // This cast is safe because Builder.CreateStore() always creates a bona fide
+ // StoreInst.
+ StoreInst *SI = cast<StoreInst>(
+ Builder.CreateStore(Vec, Builder.CreateBitCast(S0->getPointerOperand(),
+ VecTy->getPointerTo(AS))));
propagateMetadata(SI, Chain);
SI->setAlignment(Alignment);
@@ -849,7 +856,8 @@ bool Vectorizer::vectorizeStoreChain(
}
bool Vectorizer::vectorizeLoadChain(
- ArrayRef<Value *> Chain, SmallPtrSet<Value *, 16> *InstructionsProcessed) {
+ ArrayRef<Instruction *> Chain,
+ SmallPtrSet<Instruction *, 16> *InstructionsProcessed) {
LoadInst *L0 = cast<LoadInst>(Chain[0]);
// If the vector has an int element, default to int for the whole load.
@@ -877,7 +885,7 @@ bool Vectorizer::vectorizeLoadChain(
return false;
}
- ArrayRef<Value *> NewChain = getVectorizablePrefix(Chain);
+ ArrayRef<Instruction *> NewChain = getVectorizablePrefix(Chain);
if (NewChain.empty()) {
// No vectorization possible.
InstructionsProcessed->insert(Chain.begin(), Chain.end());
@@ -949,8 +957,8 @@ bool Vectorizer::vectorizeLoadChain(
DEBUG({
dbgs() << "LSV: Loads to vectorize:\n";
- for (Value *V : Chain)
- V->dump();
+ for (Instruction *I : Chain)
+ I->dump();
});
// getVectorizablePrefix already computed getBoundaryInstrs. The value of
@@ -962,7 +970,8 @@ bool Vectorizer::vectorizeLoadChain(
Value *Bitcast =
Builder.CreateBitCast(L0->getPointerOperand(), VecTy->getPointerTo(AS));
-
+ // This cast is safe because Builder.CreateLoad always creates a bona fide
+ // LoadInst.
LoadInst *LI = cast<LoadInst>(Builder.CreateLoad(Bitcast));
propagateMetadata(LI, Chain);
LI->setAlignment(Alignment);
@@ -973,17 +982,17 @@ bool Vectorizer::vectorizeLoadChain(
unsigned VecWidth = VecLoadTy->getNumElements();
for (unsigned I = 0, E = Chain.size(); I != E; ++I) {
for (auto Use : Chain[I]->users()) {
+ // All users of vector loads are ExtractElement instructions with
+ // constant indices, otherwise we would have bailed before now.
Instruction *UI = cast<Instruction>(Use);
unsigned Idx = cast<ConstantInt>(UI->getOperand(1))->getZExtValue();
unsigned NewIdx = Idx + I * VecWidth;
Value *V = Builder.CreateExtractElement(LI, Builder.getInt32(NewIdx));
- Instruction *Extracted = cast<Instruction>(V);
- if (Extracted->getType() != UI->getType())
- Extracted = cast<Instruction>(
- Builder.CreateBitCast(Extracted, UI->getType()));
+ if (V->getType() != UI->getType())
+ V = Builder.CreateBitCast(V, UI->getType());
// Replace the old instruction.
- UI->replaceAllUsesWith(Extracted);
+ UI->replaceAllUsesWith(V);
InstrsToErase.push_back(UI);
}
}
@@ -998,15 +1007,13 @@ bool Vectorizer::vectorizeLoadChain(
} else {
for (unsigned I = 0, E = Chain.size(); I != E; ++I) {
Value *V = Builder.CreateExtractElement(LI, Builder.getInt32(I));
- Instruction *Extracted = cast<Instruction>(V);
- Instruction *UI = cast<Instruction>(Chain[I]);
- if (Extracted->getType() != UI->getType()) {
- Extracted = cast<Instruction>(
- Builder.CreateBitOrPointerCast(Extracted, UI->getType()));
+ Value *CV = Chain[I];
+ if (V->getType() != CV->getType()) {
+ V = Builder.CreateBitOrPointerCast(V, CV->getType());
}
// Replace the old instruction.
- UI->replaceAllUsesWith(Extracted);
+ CV->replaceAllUsesWith(V);
}
if (Instruction *BitcastInst = dyn_cast<Instruction>(Bitcast))
More information about the llvm-commits
mailing list