diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index c9b5579c2..e2bbe7645 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -50,7 +50,7 @@ static string const one{"@ 1"}; static string const thirtyTwo{"@ 32"}; -void UnusedStoreEliminator::run(OptimiserStepContext& /*_context*/, Block& /*_ast*/) +void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast) { map functionSideEffects = SideEffectsPropagator::sideEffects( _context.dialect, @@ -87,9 +87,8 @@ void UnusedStoreEliminator::run(OptimiserStepContext& /*_context*/, Block& /*_as else rse.markActiveAsUsed(Location::Memory); rse.markActiveAsUsed(Location::Storage); - rse.scheduleUnusedForDeletion(); - StatementRemover remover(rse.m_pendingRemovals); + StatementRemover remover{rse.m_allStores - rse.m_usedStores}; remover(_ast); } @@ -192,9 +191,9 @@ void UnusedStoreEliminator::visit(Statement const& _statement) vector operations = operationsFromFunctionCall(*funCall); yulAssert(operations.size() == 1, ""); if (operations.front().location == Location::Storage) - m_activeStores["s"_yulstring].insert(&_statement); + activeStorageStores().insert(&_statement); else - m_activeStores["m"_yulstring].insert(&_statement); + activeMemoryStores().insert(&_statement); m_storeOperations[&_statement] = std::move(operations.front()); } } @@ -202,7 +201,6 @@ void UnusedStoreEliminator::visit(Statement const& _statement) void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&) { markActiveAsUsed(); - scheduleUnusedForDeletion(); } vector UnusedStoreEliminator::operationsFromFunctionCall( @@ -257,31 +255,27 @@ vector UnusedStoreEliminator::operationsFromFu void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation const& _operation) { - // TODO only one will be relevant, depending on _operation.location - for (Statement const* statement: m_activeStores["s"_yulstring]) + set toRemove; + set& active = + _operation.location == Location::Storage ? + activeStorageStores() : + activeMemoryStores(); + + // TODO this loop could be done more efficiently - removing while iterating. + for (Statement const* statement: active) { Operation const& storeOperation = m_storeOperations.at(statement); if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) - // TODO remove from active! - m_usedStores.insert(statement); - else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) { - // TODO remove from active -// state = State::Unused; - } - } - for (Statement const* statement: m_activeStores["m"_yulstring]) - { - Operation const& storeOperation = m_storeOperations.at(statement); - if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) - // TODO remove from active! + // This store is read from, mark it as used and remove it from the active set. m_usedStores.insert(statement); - else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) - { - // TODO remove from active -// state = State::Unused; + toRemove.insert(statement); } + else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) + // This store is overwritten before being read, remove it from the active set. + toRemove.insert(statement); } + active -= toRemove; } bool UnusedStoreEliminator::knownUnrelated( @@ -403,26 +397,26 @@ bool UnusedStoreEliminator::knownCovered( } void UnusedStoreEliminator::markActiveAsUsed( - optional _onlyLocation) + optional _onlyLocation +) { - // TODO it might make sense to use YulString{"m"} and YulString{"s"} for memory and storage. - // BUT: Could be both! - for (Statement const* statement: m_activeStores[YulString{}]) - if (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) + if (_onlyLocation == nullopt || _onlyLocation == Location::Memory) + for (Statement const* statement: activeMemoryStores()) m_usedStores.insert(statement); - // TODO and remove from active + if (_onlyLocation == nullopt || _onlyLocation == Location::Storage) + for (Statement const* statement: activeStorageStores()) + m_usedStores.insert(statement); + clearActive(_onlyLocation); } -void UnusedStoreEliminator::changeUndecidedTo( - State _newState, - optional _onlyLocation) +void UnusedStoreEliminator::clearActive( + optional _onlyLocation +) { - for (auto& [statement, state]: m_activeStores[YulString{}]) - if ( - state == State::Undecided && - (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) - ) - state = _newState; + if (_onlyLocation == nullopt || _onlyLocation == Location::Memory) + activeMemoryStores() = {}; + if (_onlyLocation == nullopt || _onlyLocation == Location::Storage) + activeStorageStores() = {}; } optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& _expression) const @@ -432,10 +426,3 @@ optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& return {identifier->name}; return nullopt; } - -void UnusedStoreEliminator::scheduleUnusedForDeletion() -{ - for (auto const& [statement, state]: m_activeStores[YulString{}]) - if (state == State::Unused) - m_pendingRemovals.insert(statement); -} diff --git a/libyul/optimiser/UnusedStoreEliminator.h b/libyul/optimiser/UnusedStoreEliminator.h index ce9c1fc16..94620ab7b 100644 --- a/libyul/optimiser/UnusedStoreEliminator.h +++ b/libyul/optimiser/UnusedStoreEliminator.h @@ -49,8 +49,7 @@ struct AssignedValue; * to sstore, as we don't know whether the memory location will be read once we leave the function's scope, * so the statement will be removed only if all code code paths lead to a memory overwrite. * - * The m_store member of UnusedStoreBase is only used with the empty yul string - * as key in the first dimension. + * The m_store member of UnusedStoreBase uses the key "m" for memory ond "s" for storage stores. * * Best run in SSA form. * @@ -64,12 +63,12 @@ public: explicit UnusedStoreEliminator( Dialect const& _dialect, - std::map const& ,//_functionSideEffects, - std::map,// _controlFlowSideEffects, - std::map const&,// _ssaValues, - bool// _ignoreMemory + std::map const& _functionSideEffects, + std::map _controlFlowSideEffects, + std::map const& _ssaValues, + bool _ignoreMemory ): - UnusedStoreBase(_dialect)//, + UnusedStoreBase(_dialect), m_ignoreMemory(_ignoreMemory), m_functionSideEffects(_functionSideEffects), m_controlFlowSideEffects(_controlFlowSideEffects), @@ -98,10 +97,13 @@ public: }; private: + std::set& activeMemoryStores() { return m_activeStores["m"_yulstring]; } + std::set& activeStorageStores() { return m_activeStores["m"_yulstring]; } + void shortcutNestedLoop(ActiveStores const&) override { // We might only need to do this for newly introduced stores in the loop. - changeUndecidedTo(State::Used); + markActiveAsUsed(); } void finalizeFunctionDefinition(FunctionDefinition const&) override; @@ -112,7 +114,6 @@ private: void markActiveAsUsed(std::optional _onlyLocation = std::nullopt); void clearActive(std::optional _onlyLocation = std::nullopt); - void scheduleUnusedForDeletion(); std::optional identifierNameIfSSA(Expression const& _expression) const;