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: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/Reactor/Optimizer.cpp b/src/Reactor/Optimizer.cpp
index 01721e5..3de6283 100644
--- a/src/Reactor/Optimizer.cpp
+++ b/src/Reactor/Optimizer.cpp
@@ -36,12 +36,10 @@
void analyzeUses(Ice::Cfg *function);
void eliminateDeadCode();
+ void eliminateUnitializedLoads();
void propagateAlloca();
void performScalarReplacementOfAggregates();
void optimizeSingleBasicBlockLoadsStores();
- void eliminateUnitializedLoads();
- void eliminateLoadsFollowingSingleStore();
- void optimizeStoresInSingleBasicBlock();
void replace(Ice::Inst *instruction, Ice::Operand *newValue);
void deleteInstruction(Ice::Inst *instruction);
@@ -53,7 +51,6 @@
static const Ice::InstIntrinsic *asStoreSubVector(const Ice::Inst *instruction);
static bool isLoad(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 storeTypeMatchesStore(const Ice::Inst *store1, const Ice::Inst *store2);
@@ -90,16 +87,9 @@
void setUses(Ice::Operand *, Optimizer::Uses *);
bool hasUses(Ice::Operand *) const;
- Ice::CfgNode *getNode(Ice::Inst *);
- void setNode(Ice::Inst *, Ice::CfgNode *);
-
Ice::Inst *getDefinition(Ice::Variable *);
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;
rr::Nucleus::OptimizerReport *report = nullptr;
@@ -115,6 +105,7 @@
// Start by eliminating any dead code, to avoid redundant work for the
// subsequent optimization passes.
eliminateDeadCode();
+ eliminateUnitializedLoads();
// Eliminate allocas which store the address of other allocas.
propagateAlloca();
@@ -125,11 +116,6 @@
// Iterate through basic blocks to propagate loads following stores.
optimizeSingleBasicBlockLoadsStores();
- eliminateUnitializedLoads();
- eliminateLoadsFollowingSingleStore();
- optimizeStoresInSingleBasicBlock();
- eliminateDeadCode();
-
for(auto operand : operandsWithUses)
{
// Deletes the Uses instance on the operand
@@ -396,238 +382,6 @@
}
}
-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.
void Optimizer::optimizeSingleBasicBlockLoadsStores()
{
@@ -667,7 +421,6 @@
{
Ice::Inst *previousStore = entry->second.store;
- // TODO(b/179668593): Also eliminate it if the next store is larger in size.
if(storeTypeMatchesStore(&inst, previousStore) &&
entry->second.allLoadsReplaced)
{
@@ -688,8 +441,6 @@
{
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))
{
replace(&inst, store->getData());
@@ -705,7 +456,7 @@
}
// 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();
}
@@ -720,7 +471,6 @@
continue;
}
- setNode(&instruction, basicBlock);
if(instruction.getDest())
{
setDefinition(instruction.getDest(), &instruction);
@@ -945,23 +695,6 @@
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)
{
if(!load || !store)
@@ -1072,16 +805,6 @@
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)
{
return (Ice::Inst *)var->Ice::Variable::getExternalData();
@@ -1092,21 +815,6 @@
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
{
return size() == (loads.size() + stores.size());
diff --git a/tests/ReactorUnitTests/ReactorUnitTests.cpp b/tests/ReactorUnitTests/ReactorUnitTests.cpp
index 6da82dd..e52d060 100644
--- a/tests/ReactorUnitTests/ReactorUnitTests.cpp
+++ b/tests/ReactorUnitTests/ReactorUnitTests.cpp
@@ -467,7 +467,6 @@
{
Int a = 444;
Int b = 555;
- Int c = 666;
Pointer<Int> p = &a;
Pointer<Pointer<Int>> pp = &p;
