From ddf1d023bdb7877b82fdf8f668c827f8714ad506 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 28 Nov 2022 17:10:02 +0100 Subject: [PATCH] fix it again --- libyul/optimiser/UnusedAssignEliminator.cpp | 10 +++++----- libyul/optimiser/UnusedStoreBase.cpp | 5 +++-- libyul/optimiser/UnusedStoreBase.h | 11 ++++++----- libyul/optimiser/UnusedStoreEliminator.cpp | 12 ++++++------ 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index 6bb9a0643..55a4066e5 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -45,9 +45,9 @@ void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) UnusedAssignEliminator rae{_context.dialect}; rae(_ast); - rae.m_storesToRemove += move(rae.m_potentiallyUnusedStores); + rae.m_storesToRemove += rae.m_allStores - rae.m_usedStores; - StatementRemover remover{rae.m_storesToRemove}; + StatementRemover remover{std::set{rae.m_storesToRemove.begin(), rae.m_storesToRemove.end()}}; remover(_ast); } @@ -101,7 +101,7 @@ void UnusedAssignEliminator::visit(Statement const& _statement) // by adding a test where one var is used but not the other) if (SideEffectsCollector{m_dialect, *assignment->value}.movable()) { - m_potentiallyUnusedStores.insert(&_statement); + m_allStores.insert(&_statement); for (auto const& var: assignment->variableNames) m_activeStores[var.name] = {&_statement}; } @@ -135,7 +135,7 @@ void UnusedAssignEliminator::shortcutNestedLoop(ActiveStores const& _zeroRuns) auto zeroIt = _zeroRuns.find(variable); if (zeroIt != _zeroRuns.end() && zeroIt->second.count(assignment)) continue; - m_potentiallyUnusedStores.erase(assignment); + m_usedStores.insert(assignment); } } @@ -148,6 +148,6 @@ void UnusedAssignEliminator::finalizeFunctionDefinition(FunctionDefinition const void UnusedAssignEliminator::markUsed(YulString _variable) { for (auto& assignment: m_activeStores[_variable]) - m_potentiallyUnusedStores.erase(assignment); + m_usedStores.insert(assignment); m_activeStores.erase(_variable); } diff --git a/libyul/optimiser/UnusedStoreBase.cpp b/libyul/optimiser/UnusedStoreBase.cpp index 2d70d74b7..4a5123b6f 100644 --- a/libyul/optimiser/UnusedStoreBase.cpp +++ b/libyul/optimiser/UnusedStoreBase.cpp @@ -71,15 +71,16 @@ void UnusedStoreBase::operator()(Switch const& _switch) void UnusedStoreBase::operator()(FunctionDefinition const& _functionDefinition) { + ScopedSaveAndRestore allStores(m_allStores, {}); + ScopedSaveAndRestore usedStoresStores(m_usedStores, {}); 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); + m_storesToRemove += m_allStores - m_usedStores; } void UnusedStoreBase::operator()(ForLoop const& _forLoop) diff --git a/libyul/optimiser/UnusedStoreBase.h b/libyul/optimiser/UnusedStoreBase.h index 563d28677..7568620f7 100644 --- a/libyul/optimiser/UnusedStoreBase.h +++ b/libyul/optimiser/UnusedStoreBase.h @@ -74,11 +74,12 @@ protected: static void merge(ActiveStores& _target, std::vector&& _source); Dialect const& m_dialect; - /// 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; + /// Set of all stores encountered during the traversal (in the current function). + std::set m_allStores; + /// Set of stores that are marked as being used (in the current function). + std::set m_usedStores; + /// List of stores that can be removed (globally). + std::vector 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 09e946f58..2ba0ec3d0 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -85,9 +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); + rse.m_storesToRemove += rse.m_allStores - rse.m_usedStores; - StatementRemover remover{rse.m_storesToRemove}; + StatementRemover remover{std::set{rse.m_storesToRemove.begin(), rse.m_storesToRemove.end()}}; remover(_ast); } @@ -191,7 +191,7 @@ void UnusedStoreEliminator::visit(Statement const& _statement) if (!allowReturndatacopyToBeRemoved) return; } - m_potentiallyUnusedStores.insert(&_statement); + m_allStores.insert(&_statement); vector operations = operationsFromFunctionCall(*funCall); yulAssert(operations.size() == 1, ""); if (operations.front().location == Location::Storage) @@ -268,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_potentiallyUnusedStores.erase(statement); + m_usedStores.insert(statement); it = active.erase(it); } else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) @@ -403,10 +403,10 @@ void UnusedStoreEliminator::markActiveAsUsed( { if (_onlyLocation == nullopt || _onlyLocation == Location::Memory) for (Statement const* statement: activeMemoryStores()) - m_potentiallyUnusedStores.erase(statement); + m_usedStores.insert(statement); if (_onlyLocation == nullopt || _onlyLocation == Location::Storage) for (Statement const* statement: activeStorageStores()) - m_potentiallyUnusedStores.erase(statement); + m_usedStores.insert(statement); clearActive(_onlyLocation); }