[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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@276938 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Justin Lebar 2016-07-27 23:06:00 +00:00
parent b3f897d0e3
commit 741862c996

View File

@ -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::createLoadStoreVectorizerPass() {
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<Value *> Chain) {
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<Value *> Chain,
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::getVectorizablePrefix(ArrayRef<Value *> Chain) {
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::getVectorizablePrefix(ArrayRef<Value *> Chain) {
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::getVectorizablePrefix(ArrayRef<Value *> Chain) {
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(BasicBlock *BB) {
// 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(BasicBlock *BB) {
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(BasicBlock *BB) {
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(ValueListMap &Map) {
// 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(ValueListMap &Map) {
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(ArrayRef<Value *> Instrs) {
}
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(ArrayRef<Value *> Instrs) {
// 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(ArrayRef<Value *> Instrs) {
}
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))