Skip to content

S
swiftshader
Commit 6ddeb6ba by Nicolas Capens Committed by Nicolas Capens
Options

Remove legacy optimization passes.

optimizeSingleBasicBlockLoadsStores() supersedes both eliminateLoadsFollowingSingleStore() and optimizeStoresInSingleBasicBlock(). This was verified by adding asserts the latter when the delete more instructions. They're never hit by dEQP-VK tests. Only the PointerToPointer triggers an assert. It goes away when running optimizeSingleBasicBlockLoadsStores() twice. Since it is very rare to store the address of a pointer in another pointer, and before the new optimization pass was implemented we also did not handle this case, we're not going to pay the cost of running it twice. eliminateUnitializedLoads() was moved earlier because we never produce new loads of uninitialized values in the optimization passes and it's best to eliminate these allocas early. The late eliminateDeadCode() was also removed since we already run it at the end of optimizeSingleBasicBlockLoadsStores(). TODOs for cases where the load/store sizes don't match have been removed because propagating a different type causes issues. Bug: b/179668593 Change-Id: I4b3533057423709cdaa8343301184d8225b0727b Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/53128 Presubmit-Ready: Nicolas Capens <nicolascapens@google.com> Kokoro-Result: kokoro <noreply+kokoro@google.com> Tested-by: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarAntonio Maiorano <amaiorano@google.com>
parent a7dfb849
Showing with 3 additions and 296 deletions
src/Reactor/Optimizer.cpp
...@@ -36,12 +36,10 @@ private: ...@@ -36,12 +36,10 @@ private:
void analyzeUses(Ice::Cfg *function); void analyzeUses(Ice::Cfg *function);
void eliminateDeadCode(); void eliminateDeadCode();
void eliminateUnitializedLoads();
void propagateAlloca(); void propagateAlloca();
void performScalarReplacementOfAggregates(); void performScalarReplacementOfAggregates();
void optimizeSingleBasicBlockLoadsStores(); void optimizeSingleBasicBlockLoadsStores();
void eliminateUnitializedLoads();
void eliminateLoadsFollowingSingleStore();
void optimizeStoresInSingleBasicBlock();
void replace(Ice::Inst *instruction, Ice::Operand *newValue); void replace(Ice::Inst *instruction, Ice::Operand *newValue);
void deleteInstruction(Ice::Inst *instruction); void deleteInstruction(Ice::Inst *instruction);
...@@ -53,7 +51,6 @@ private: ...@@ -53,7 +51,6 @@ private:
static const Ice::InstIntrinsic *asStoreSubVector(const Ice::Inst *instruction); static const Ice::InstIntrinsic *asStoreSubVector(const Ice::Inst *instruction);
static bool isLoad(const Ice::Inst &instruction); static bool isLoad(const Ice::Inst &instruction);
static bool isStore(const Ice::Inst &instruction); static bool isStore(const Ice::Inst &instruction);
static std::size_t storeSize(const Ice::Inst *instruction);
static bool loadTypeMatchesStore(const Ice::Inst *load, const Ice::Inst *store); static bool loadTypeMatchesStore(const Ice::Inst *load, const Ice::Inst *store);
static bool storeTypeMatchesStore(const Ice::Inst *store1, const Ice::Inst *store2); static bool storeTypeMatchesStore(const Ice::Inst *store1, const Ice::Inst *store2);
...@@ -90,16 +87,9 @@ private: ...@@ -90,16 +87,9 @@ private:
void setUses(Ice::Operand *, Optimizer::Uses *); void setUses(Ice::Operand *, Optimizer::Uses *);
bool hasUses(Ice::Operand *) const; bool hasUses(Ice::Operand *) const;
Ice::CfgNode *getNode(Ice::Inst *);
void setNode(Ice::Inst *, Ice::CfgNode *);
Ice::Inst *getDefinition(Ice::Variable *); Ice::Inst *getDefinition(Ice::Variable *);
void setDefinition(Ice::Variable *, Ice::Inst *); void setDefinition(Ice::Variable *, Ice::Inst *);
const std::vector<LoadStoreInst> &getLoadStoreInsts(Ice::CfgNode *);
void setLoadStoreInsts(Ice::CfgNode *, std::vector<LoadStoreInst> *);
bool hasLoadStoreInsts(Ice::CfgNode *node) const;
std::vector<Ice::Operand *> operandsWithUses; std::vector<Ice::Operand *> operandsWithUses;
rr::Nucleus::OptimizerReport *report = nullptr; rr::Nucleus::OptimizerReport *report = nullptr;
...@@ -115,6 +105,7 @@ void Optimizer::run(Ice::Cfg *function) ...@@ -115,6 +105,7 @@ void Optimizer::run(Ice::Cfg *function)
// Start by eliminating any dead code, to avoid redundant work for the // Start by eliminating any dead code, to avoid redundant work for the
// subsequent optimization passes. // subsequent optimization passes.
eliminateDeadCode(); eliminateDeadCode();
eliminateUnitializedLoads();
// Eliminate allocas which store the address of other allocas. // Eliminate allocas which store the address of other allocas.
propagateAlloca(); propagateAlloca();
...@@ -125,11 +116,6 @@ void Optimizer::run(Ice::Cfg *function) ...@@ -125,11 +116,6 @@ void Optimizer::run(Ice::Cfg *function)
// Iterate through basic blocks to propagate loads following stores. // Iterate through basic blocks to propagate loads following stores.
optimizeSingleBasicBlockLoadsStores(); optimizeSingleBasicBlockLoadsStores();
eliminateUnitializedLoads();
eliminateLoadsFollowingSingleStore();
optimizeStoresInSingleBasicBlock();
eliminateDeadCode();
for(auto operand : operandsWithUses) for(auto operand : operandsWithUses)
{ {
// Deletes the Uses instance on the operand // Deletes the Uses instance on the operand
...@@ -396,238 +382,6 @@ void Optimizer::eliminateUnitializedLoads() ...@@ -396,238 +382,6 @@ void Optimizer::eliminateUnitializedLoads()
} }
} }
void Optimizer::eliminateLoadsFollowingSingleStore()
{
Ice::CfgNode *entryBlock = function->getEntryNode();
for(Ice::Inst &alloca : entryBlock->getInsts())
{
if(alloca.isDeleted())
{
continue;
}
if(!llvm::isa<Ice::InstAlloca>(alloca))
{
break; // Allocas are all at the top
}
Ice::Operand *address = alloca.getDest();
if(!hasUses(address))
{
continue;
}
auto &addressUses = *getUses(address);
if(!addressUses.areOnlyLoadStore())
{
continue;
}
if(addressUses.stores.size() == 1)
{
Ice::Inst *store = addressUses.stores[0];
Ice::Operand *storeValue = store->getData();
auto instIterator = store->getIterator();
auto basicBlockEnd = getNode(store)->getInsts().end();
while(++instIterator != basicBlockEnd)
{
Ice::Inst *load = &*instIterator;
if(load->isDeleted() || !isLoad(*load))
{
continue;
}
if(load->getLoadAddress() != address)
{
continue;
}
if(!loadTypeMatchesStore(load, store))
{
continue;
}
replace(load, storeValue);
for(size_t i = 0; i < addressUses.loads.size(); i++)
{
if(addressUses.loads[i] == load)
{
addressUses.loads[i] = addressUses.loads.back();
addressUses.loads.pop_back();
break;
}
}
for(size_t i = 0; i < addressUses.size(); i++)
{
if(addressUses[i] == load)
{
addressUses[i] = addressUses.back();
addressUses.pop_back();
break;
}
}
if(addressUses.size() == 1)
{
assert(addressUses[0] == store);
alloca.setDeleted();
store->setDeleted();
setUses(address, nullptr);
if(hasUses(storeValue))
{
auto &valueUses = *getUses(storeValue);
for(size_t i = 0; i < valueUses.size(); i++)
{
if(valueUses[i] == store)
{
valueUses[i] = valueUses.back();
valueUses.pop_back();
break;
}
}
if(valueUses.empty())
{
setUses(storeValue, nullptr);
}
}
break;
}
}
}
}
}
void Optimizer::optimizeStoresInSingleBasicBlock()
{
Ice::CfgNode *entryBlock = function->getEntryNode();
std::vector<std::vector<LoadStoreInst> *> allocatedVectors;
for(Ice::Inst &alloca : entryBlock->getInsts())
{
if(alloca.isDeleted())
{
continue;
}
if(!llvm::isa<Ice::InstAlloca>(alloca))
{
break; // Allocas are all at the top
}
Ice::Operand *address = alloca.getDest();
if(!hasUses(address))
{
continue;
}
const auto &addressUses = *getUses(address);
if(!addressUses.areOnlyLoadStore())
{
continue;
}
Ice::CfgNode *singleBasicBlock = getNode(addressUses.stores[0]);
for(size_t i = 1; i < addressUses.stores.size(); i++)
{
Ice::Inst *store = addressUses.stores[i];
if(getNode(store) != singleBasicBlock)
{
singleBasicBlock = nullptr;
break;
}
}
if(singleBasicBlock)
{
if(!hasLoadStoreInsts(singleBasicBlock))
{
std::vector<LoadStoreInst> *loadStoreInstVector = new std::vector<LoadStoreInst>();
setLoadStoreInsts(singleBasicBlock, loadStoreInstVector);
allocatedVectors.push_back(loadStoreInstVector);
for(Ice::Inst &inst : singleBasicBlock->getInsts())
{
if(inst.isDeleted())
{
continue;
}
bool isStoreInst = isStore(inst);
bool isLoadInst = isLoad(inst);
if(isStoreInst || isLoadInst)
{
loadStoreInstVector->push_back(LoadStoreInst(&inst, isStoreInst));
}
}
}
Ice::Inst *store = nullptr;
Ice::Operand *storeValue = nullptr;
bool unmatchedLoads = false;
for(auto &loadStoreInst : getLoadStoreInsts(singleBasicBlock))
{
Ice::Inst *inst = loadStoreInst.inst;
if((loadStoreInst.address != address) || inst->isDeleted())
{
continue;
}
if(loadStoreInst.isStore)
{
// New store found. If we had a previous one, try to eliminate it.
if(store && !unmatchedLoads)
{
// If the previous store is wider than the new one, we can't eliminate it
// because there could be a wide load reading its non-overwritten data.
if(storeSize(inst) >= storeSize(store))
{
deleteInstruction(store);
}
}
store = inst;
storeValue = store->getData();
unmatchedLoads = false;
}
else
{
if(!loadTypeMatchesStore(inst, store))
{
unmatchedLoads = true;
continue;
}
replace(inst, storeValue);
}
}
}
}
for(auto loadStoreInstVector : allocatedVectors)
{
delete loadStoreInstVector;
}
}
// Iterate through basic blocks to propagate stores to subsequent loads. // Iterate through basic blocks to propagate stores to subsequent loads.
void Optimizer::optimizeSingleBasicBlockLoadsStores() void Optimizer::optimizeSingleBasicBlockLoadsStores()
{ {
...@@ -667,7 +421,6 @@ void Optimizer::optimizeSingleBasicBlockLoadsStores() ...@@ -667,7 +421,6 @@ void Optimizer::optimizeSingleBasicBlockLoadsStores()
{ {
Ice::Inst *previousStore = entry->second.store; Ice::Inst *previousStore = entry->second.store;
// TODO(b/179668593): Also eliminate it if the next store is larger in size.
if(storeTypeMatchesStore(&inst, previousStore) && if(storeTypeMatchesStore(&inst, previousStore) &&
entry->second.allLoadsReplaced) entry->second.allLoadsReplaced)
{ {
...@@ -688,8 +441,6 @@ void Optimizer::optimizeSingleBasicBlockLoadsStores() ...@@ -688,8 +441,6 @@ void Optimizer::optimizeSingleBasicBlockLoadsStores()
{ {
const Ice::Inst *store = entry->second.store; const Ice::Inst *store = entry->second.store;
// Conservatively check that the types match.
// TODO(b/179668593): Also valid to propagate if the load is smaller or equal in size to the store.
if(loadTypeMatchesStore(&inst, store)) if(loadTypeMatchesStore(&inst, store))
{ {
replace(&inst, store->getData()); replace(&inst, store->getData());
...@@ -705,7 +456,7 @@ void Optimizer::optimizeSingleBasicBlockLoadsStores() ...@@ -705,7 +456,7 @@ void Optimizer::optimizeSingleBasicBlockLoadsStores()
} }
// This can leave some dead instructions. Specifically stores. // This can leave some dead instructions. Specifically stores.
// TODO((b/179668593): Eliminate stores superseded by subsequent stores? // TODO(b/179668593): Check just for dead stores by iterating over allocas?
eliminateDeadCode(); eliminateDeadCode();
} }
...@@ -720,7 +471,6 @@ void Optimizer::analyzeUses(Ice::Cfg *function) ...@@ -720,7 +471,6 @@ void Optimizer::analyzeUses(Ice::Cfg *function)
continue; continue;
} }
setNode(&instruction, basicBlock);
if(instruction.getDest()) if(instruction.getDest())
{ {
setDefinition(instruction.getDest(), &instruction); setDefinition(instruction.getDest(), &instruction);
...@@ -945,23 +695,6 @@ bool Optimizer::isStore(const Ice::Inst &instruction) ...@@ -945,23 +695,6 @@ bool Optimizer::isStore(const Ice::Inst &instruction)
return asStoreSubVector(&instruction) != nullptr; return asStoreSubVector(&instruction) != nullptr;
} }
std::size_t Optimizer::storeSize(const Ice::Inst *store)
{
assert(isStore(*store));
if(auto *instStore = llvm::dyn_cast<Ice::InstStore>(store))
{
return Ice::typeWidthInBytes(instStore->getData()->getType());
}
if(auto *storeSubVector = asStoreSubVector(store))
{
return llvm::cast<Ice::ConstantInteger32>(storeSubVector->getSrc(3))->getValue();
}
return 0;
}
bool Optimizer::loadTypeMatchesStore(const Ice::Inst *load, const Ice::Inst *store) bool Optimizer::loadTypeMatchesStore(const Ice::Inst *load, const Ice::Inst *store)
{ {
if(!load || !store) if(!load || !store)
...@@ -1072,16 +805,6 @@ bool Optimizer::hasUses(Ice::Operand *operand) const ...@@ -1072,16 +805,6 @@ bool Optimizer::hasUses(Ice::Operand *operand) const
return operand->Ice::Operand::getExternalData() != nullptr; return operand->Ice::Operand::getExternalData() != nullptr;
} }
Ice::CfgNode *Optimizer::getNode(Ice::Inst *inst)
{
return (Ice::CfgNode *)inst->Ice::Inst::getExternalData();
}
void Optimizer::setNode(Ice::Inst *inst, Ice::CfgNode *node)
{
inst->Ice::Inst::setExternalData(node);
}
Ice::Inst *Optimizer::getDefinition(Ice::Variable *var) Ice::Inst *Optimizer::getDefinition(Ice::Variable *var)
{ {
return (Ice::Inst *)var->Ice::Variable::getExternalData(); return (Ice::Inst *)var->Ice::Variable::getExternalData();
...@@ -1092,21 +815,6 @@ void Optimizer::setDefinition(Ice::Variable *var, Ice::Inst *inst) ...@@ -1092,21 +815,6 @@ void Optimizer::setDefinition(Ice::Variable *var, Ice::Inst *inst)
var->Ice::Variable::setExternalData(inst); var->Ice::Variable::setExternalData(inst);
} }
const std::vector<Optimizer::LoadStoreInst> &Optimizer::getLoadStoreInsts(Ice::CfgNode *node)
{
return *((const std::vector<LoadStoreInst> *)node->Ice::CfgNode::getExternalData());
}
void Optimizer::setLoadStoreInsts(Ice::CfgNode *node, std::vector<LoadStoreInst> *insts)
{
node->Ice::CfgNode::setExternalData(insts);
}
bool Optimizer::hasLoadStoreInsts(Ice::CfgNode *node) const
{
return node->Ice::CfgNode::getExternalData() != nullptr;
}
bool Optimizer::Uses::areOnlyLoadStore() const bool Optimizer::Uses::areOnlyLoadStore() const
{ {
return size() == (loads.size() + stores.size()); return size() == (loads.size() + stores.size());
......
tests/ReactorUnitTests/ReactorUnitTests.cpp
...@@ -467,7 +467,6 @@ TEST(ReactorUnitTests, PointerToPointer) ...@@ -467,7 +467,6 @@ TEST(ReactorUnitTests, PointerToPointer)
{ {
Int a = 444; Int a = 444;
Int b = 555; Int b = 555;
Int c = 666;
Pointer<Int> p = &a; Pointer<Int> p = &a;
Pointer<Pointer<Int>> pp = &p; Pointer<Pointer<Int>> pp = &p;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment