From d89c5638f02092dc3a9edd75e323cdd4c51db8ce Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 28 Nov 2022 16:30:32 +0100 Subject: [PATCH] more work --- libyul/optimiser/UnusedAssignEliminator.cpp | 44 ++++++++------------- libyul/optimiser/UnusedAssignEliminator.h | 2 - libyul/optimiser/UnusedStoreBase.cpp | 2 + libyul/optimiser/UnusedStoreBase.h | 9 +++-- libyul/optimiser/UnusedStoreEliminator.cpp | 11 +++--- 5 files changed, 30 insertions(+), 38 deletions(-) diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index eaa076fed..6bb9a0643 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -45,17 +45,9 @@ void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) UnusedAssignEliminator rae{_context.dialect}; rae(_ast); - set toRemove; - for (Statement const* unusedStore: rae.m_allStores - rae.m_usedStores) - // TODO this should also use user function side effects. - // Then we have to modify the multi-assign test (or verify that it is fine after all - // by adding a test where one var is used but not the other) - if (SideEffectsCollector{_context.dialect, *std::get(*unusedStore).value}.movable()) - toRemove.insert(unusedStore); - else - cerr << "not used because not movable" << endl; + rae.m_storesToRemove += move(rae.m_potentiallyUnusedStores); - StatementRemover remover{toRemove}; + StatementRemover remover{rae.m_storesToRemove}; remover(_ast); } @@ -64,14 +56,6 @@ void UnusedAssignEliminator::operator()(Identifier const& _identifier) markUsed(_identifier.name); } -void UnusedAssignEliminator::operator()(VariableDeclaration const& _variableDeclaration) -{ - UnusedStoreBase::operator()(_variableDeclaration); - - for (auto const& var: _variableDeclaration.variables) - m_declaredVariables.emplace(var.name); -} - void UnusedAssignEliminator::operator()(Assignment const& _assignment) { visit(*_assignment.value); @@ -81,7 +65,6 @@ void UnusedAssignEliminator::operator()(Assignment const& _assignment) void UnusedAssignEliminator::operator()(FunctionDefinition const& _functionDefinition) { - ScopedSaveAndRestore outerDeclaredVariables(m_declaredVariables, {}); ScopedSaveAndRestore outerReturnVariables(m_returnVariables, {}); for (auto const& retParam: _functionDefinition.returnVariables) @@ -98,10 +81,9 @@ void UnusedAssignEliminator::operator()(Leave const&) void UnusedAssignEliminator::operator()(Block const& _block) { - ScopedSaveAndRestore outerDeclaredVariables(m_declaredVariables, {}); - UnusedStoreBase::operator()(_block); + // TODO we could also move some statements from "potentially" to "toRemove". for (auto const& statement: _block.statements) if (auto const* varDecl = get_if(&statement)) for (auto const& var: varDecl->variables) @@ -114,9 +96,18 @@ void UnusedAssignEliminator::visit(Statement const& _statement) if (auto const* assignment = get_if(&_statement)) { - m_allStores.insert(&_statement); - for (auto const& var: assignment->variableNames) - m_activeStores[var.name] = {&_statement}; + // TODO this should also use user function side effects. + // Then we have to modify the multi-assign test (or verify that it is fine after all + // by adding a test where one var is used but not the other) + if (SideEffectsCollector{m_dialect, *assignment->value}.movable()) + { + m_potentiallyUnusedStores.insert(&_statement); + for (auto const& var: assignment->variableNames) + m_activeStores[var.name] = {&_statement}; + } + else + for (auto const& var: assignment->variableNames) + m_activeStores[var.name].clear(); } // cerr << "After " << std::visit(AsmPrinter{}, _statement) << endl; @@ -144,7 +135,7 @@ void UnusedAssignEliminator::shortcutNestedLoop(ActiveStores const& _zeroRuns) auto zeroIt = _zeroRuns.find(variable); if (zeroIt != _zeroRuns.end() && zeroIt->second.count(assignment)) continue; - m_usedStores.insert(assignment); + m_potentiallyUnusedStores.erase(assignment); } } @@ -157,7 +148,6 @@ void UnusedAssignEliminator::finalizeFunctionDefinition(FunctionDefinition const void UnusedAssignEliminator::markUsed(YulString _variable) { for (auto& assignment: m_activeStores[_variable]) - m_usedStores.insert(assignment); - // TODO is this correct? + m_potentiallyUnusedStores.erase(assignment); m_activeStores.erase(_variable); } diff --git a/libyul/optimiser/UnusedAssignEliminator.h b/libyul/optimiser/UnusedAssignEliminator.h index d90278b77..75d9659c4 100644 --- a/libyul/optimiser/UnusedAssignEliminator.h +++ b/libyul/optimiser/UnusedAssignEliminator.h @@ -116,7 +116,6 @@ public: explicit UnusedAssignEliminator(Dialect const& _dialect): UnusedStoreBase(_dialect) {} void operator()(Identifier const& _identifier) override; - void operator()(VariableDeclaration const& _variableDeclaration) override; void operator()(Assignment const& _assignment) override; void operator()(FunctionDefinition const&) override; void operator()(Leave const&) override; @@ -131,7 +130,6 @@ private: void markUsed(YulString _variable); - std::set m_declaredVariables; std::set m_returnVariables; }; diff --git a/libyul/optimiser/UnusedStoreBase.cpp b/libyul/optimiser/UnusedStoreBase.cpp index 2f1c0a5da..2d70d74b7 100644 --- a/libyul/optimiser/UnusedStoreBase.cpp +++ b/libyul/optimiser/UnusedStoreBase.cpp @@ -74,10 +74,12 @@ void UnusedStoreBase::operator()(FunctionDefinition const& _functionDefinition) ScopedSaveAndRestore outerAssignments(m_activeStores, {}); ScopedSaveAndRestore forLoopInfo(m_forLoopInfo, {}); ScopedSaveAndRestore forLoopNestingDepth(m_forLoopNestingDepth, 0); + ScopedSaveAndRestore potentiallyUnused(m_potentiallyUnusedStores, {}); (*this)(_functionDefinition.body); finalizeFunctionDefinition(_functionDefinition); + m_storesToRemove += move(m_potentiallyUnusedStores); } void UnusedStoreBase::operator()(ForLoop const& _forLoop) diff --git a/libyul/optimiser/UnusedStoreBase.h b/libyul/optimiser/UnusedStoreBase.h index 35989da01..563d28677 100644 --- a/libyul/optimiser/UnusedStoreBase.h +++ b/libyul/optimiser/UnusedStoreBase.h @@ -74,10 +74,11 @@ protected: static void merge(ActiveStores& _target, std::vector&& _source); Dialect const& m_dialect; - /// Set of all stores encountered during the traversal - std::set m_allStores; - /// Set of stores that are marked as being used. - std::set m_usedStores; + /// Set of stores that unused. Once a store is deemed used, it is removed from here. + std::set m_potentiallyUnusedStores; + /// Statements are moved from m_potentiallUnusedStores to m_storesToRemove at the + /// end of each function. + std::set m_storesToRemove; /// Active (undecided) stores in the current branch. ActiveStores m_activeStores; diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 9fa15f2ca..09e946f58 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -85,8 +85,9 @@ void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast) else rse.markActiveAsUsed(Location::Memory); rse.markActiveAsUsed(Location::Storage); + rse.m_storesToRemove += move(rse.m_potentiallyUnusedStores); - StatementRemover remover{rse.m_allStores - rse.m_usedStores}; + StatementRemover remover{rse.m_storesToRemove}; remover(_ast); } @@ -190,7 +191,7 @@ void UnusedStoreEliminator::visit(Statement const& _statement) if (!allowReturndatacopyToBeRemoved) return; } - m_allStores.insert(&_statement); + m_potentiallyUnusedStores.insert(&_statement); vector operations = operationsFromFunctionCall(*funCall); yulAssert(operations.size() == 1, ""); if (operations.front().location == Location::Storage) @@ -267,7 +268,7 @@ void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation cons if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) { // This store is read from, mark it as used and remove it from the active set. - m_usedStores.insert(statement); + m_potentiallyUnusedStores.erase(statement); it = active.erase(it); } else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) @@ -402,10 +403,10 @@ void UnusedStoreEliminator::markActiveAsUsed( { if (_onlyLocation == nullopt || _onlyLocation == Location::Memory) for (Statement const* statement: activeMemoryStores()) - m_usedStores.insert(statement); + m_potentiallyUnusedStores.erase(statement); if (_onlyLocation == nullopt || _onlyLocation == Location::Storage) for (Statement const* statement: activeStorageStores()) - m_usedStores.insert(statement); + m_potentiallyUnusedStores.erase(statement); clearActive(_onlyLocation); }