From bd7676873e34957d4d865cd92cd354c0fbab3ad8 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 23 Nov 2022 15:28:11 +0100 Subject: [PATCH] modify unused store --- libyul/optimiser/UnusedAssignEliminator.cpp | 95 +-- libyul/optimiser/UnusedAssignEliminator.h | 9 +- libyul/optimiser/UnusedStoreBase.cpp | 52 +- libyul/optimiser/UnusedStoreBase.h | 36 +- libyul/optimiser/UnusedStoreEliminator.cpp | 638 +++++++++--------- libyul/optimiser/UnusedStoreEliminator.h | 84 +-- .../for_deep_noremove.yul | 5 +- 7 files changed, 452 insertions(+), 467 deletions(-) diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index 273aa6b79..5da7b8ae6 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -25,27 +25,40 @@ #include #include #include +#include #include #include +#include + using namespace std; using namespace solidity; using namespace solidity::yul; +// TODO this component does not handle reverting function calls specially. Is that OK? +// We should set m_activeStores to empty set for a reverting function call, like wo do with `leave`. + void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) { UnusedAssignEliminator rae{_context.dialect}; rae(_ast); - StatementRemover remover{rae.m_pendingRemovals}; + set toRemove; + for (Statement const* unusedStore: rae.m_allStores - rae.m_usedStores) + if (SideEffectsCollector{_context.dialect, *std::get(*unusedStore).value}.movable()) + toRemove.insert(unusedStore); + else + cerr << "not used because not movable" << endl; + + StatementRemover remover{toRemove}; remover(_ast); } void UnusedAssignEliminator::operator()(Identifier const& _identifier) { - changeUndecidedTo(_identifier.name, State::Used); + markUsed(_identifier.name); } void UnusedAssignEliminator::operator()(VariableDeclaration const& _variableDeclaration) @@ -59,10 +72,10 @@ void UnusedAssignEliminator::operator()(VariableDeclaration const& _variableDecl void UnusedAssignEliminator::operator()(Assignment const& _assignment) { visit(*_assignment.value); - for (auto const& var: _assignment.variableNames) - changeUndecidedTo(var.name, State::Unused); + // Do not visit the variables because they are Identifiers } + void UnusedAssignEliminator::operator()(FunctionDefinition const& _functionDefinition) { ScopedSaveAndRestore outerDeclaredVariables(m_declaredVariables, {}); @@ -77,7 +90,7 @@ void UnusedAssignEliminator::operator()(FunctionDefinition const& _functionDefin void UnusedAssignEliminator::operator()(Leave const&) { for (YulString name: m_returnVariables) - changeUndecidedTo(name, State::Used); + markUsed(name); } void UnusedAssignEliminator::operator()(Block const& _block) @@ -86,8 +99,10 @@ void UnusedAssignEliminator::operator()(Block const& _block) UnusedStoreBase::operator()(_block); - for (auto const& var: m_declaredVariables) - finalize(var, State::Unused); + for (auto const& statement: _block.statements) + if (auto const* varDecl = get_if(&statement)) + for (auto const& var: varDecl->variables) + m_activeStores.erase(var.name); } void UnusedAssignEliminator::visit(Statement const& _statement) @@ -95,63 +110,53 @@ void UnusedAssignEliminator::visit(Statement const& _statement) UnusedStoreBase::visit(_statement); if (auto const* assignment = get_if(&_statement)) - if (assignment->variableNames.size() == 1) - // Default-construct it in "Undecided" state if it does not yet exist. - m_stores[assignment->variableNames.front().name][&_statement]; + { + // TODO is it OK to do this for multi-assignments? I guess so because it is enough if + // one of them is used. + m_allStores.insert(&_statement); + for (auto const& var: assignment->variableNames) + m_activeStores[var.name] = {&_statement}; + } + +// cerr << "After " << std::visit(AsmPrinter{}, _statement) << endl; +// for (auto&& [var, assigns]: m_activeStores) +// { +// cerr << " " << var.str() << ":" << endl; +// for (auto const& assign: assigns) +// cerr << " " << std::visit(AsmPrinter{}, *assign) << endl; +// } } -void UnusedAssignEliminator::shortcutNestedLoop(TrackedStores const& _zeroRuns) +void UnusedAssignEliminator::shortcutNestedLoop(ActiveStores const& _zeroRuns) { // Shortcut to avoid horrible runtime: // Change all assignments that were newly introduced in the for loop to "used". // We do not have to do that with the "break" or "continue" paths, because // they will be joined later anyway. // TODO parallel traversal might be more efficient here. - for (auto& [variable, stores]: m_stores) + + // TODO is this correct? + + for (auto& [variable, stores]: m_activeStores) for (auto& assignment: stores) { auto zeroIt = _zeroRuns.find(variable); - if (zeroIt != _zeroRuns.end() && zeroIt->second.count(assignment.first)) + if (zeroIt != _zeroRuns.end() && zeroIt->second.count(assignment)) continue; - assignment.second = State::Value::Used; + m_usedStores.insert(assignment); } } void UnusedAssignEliminator::finalizeFunctionDefinition(FunctionDefinition const& _functionDefinition) { - for (auto const& param: _functionDefinition.parameters) - finalize(param.name, State::Unused); for (auto const& retParam: _functionDefinition.returnVariables) - finalize(retParam.name, State::Used); + markUsed(retParam.name); } -void UnusedAssignEliminator::changeUndecidedTo(YulString _variable, UnusedAssignEliminator::State _newState) +void UnusedAssignEliminator::markUsed(YulString _variable) { - for (auto& assignment: m_stores[_variable]) - if (assignment.second == State::Undecided) - assignment.second = _newState; -} - -void UnusedAssignEliminator::finalize(YulString _variable, UnusedAssignEliminator::State _finalState) -{ - std::map stores = std::move(m_stores[_variable]); - m_stores.erase(_variable); - - for (auto& breakAssignments: m_forLoopInfo.pendingBreakStmts) - { - util::joinMap(stores, std::move(breakAssignments[_variable]), State::join); - breakAssignments.erase(_variable); - } - for (auto& continueAssignments: m_forLoopInfo.pendingContinueStmts) - { - util::joinMap(stores, std::move(continueAssignments[_variable]), State::join); - continueAssignments.erase(_variable); - } - - for (auto&& [statement, state]: stores) - if ( - (state == State::Unused || (state == State::Undecided && _finalState == State::Unused)) && - SideEffectsCollector{m_dialect, *std::get(*statement).value}.movable() - ) - m_pendingRemovals.insert(statement); + for (auto& assignment: m_activeStores[_variable]) + m_usedStores.insert(assignment); + // TODO is this correct? + m_activeStores.erase(_variable); } diff --git a/libyul/optimiser/UnusedAssignEliminator.h b/libyul/optimiser/UnusedAssignEliminator.h index b5a2998a1..d90278b77 100644 --- a/libyul/optimiser/UnusedAssignEliminator.h +++ b/libyul/optimiser/UnusedAssignEliminator.h @@ -126,15 +126,10 @@ public: void visit(Statement const& _statement) override; private: - void shortcutNestedLoop(TrackedStores const& _beforeLoop) override; + void shortcutNestedLoop(ActiveStores const& _beforeLoop) override; void finalizeFunctionDefinition(FunctionDefinition const& _functionDefinition) override; - void changeUndecidedTo(YulString _variable, State _newState); - /// Called when a variable goes out of scope. Sets the state of all still undecided - /// assignments to the final state. In this case, this also applies to pending - /// break and continue TrackedStores. - void finalize(YulString _variable, State _finalState); - + 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 49f550842..2f1c0a5da 100644 --- a/libyul/optimiser/UnusedStoreBase.cpp +++ b/libyul/optimiser/UnusedStoreBase.cpp @@ -37,41 +37,41 @@ void UnusedStoreBase::operator()(If const& _if) { visit(*_if.condition); - TrackedStores skipBranch{m_stores}; + ActiveStores skipBranch{m_activeStores}; (*this)(_if.body); - merge(m_stores, std::move(skipBranch)); + merge(m_activeStores, std::move(skipBranch)); } void UnusedStoreBase::operator()(Switch const& _switch) { visit(*_switch.expression); - TrackedStores const preState{m_stores}; + ActiveStores const preState{m_activeStores}; bool hasDefault = false; - vector branches; + vector branches; for (auto const& c: _switch.cases) { if (!c.value) hasDefault = true; (*this)(c.body); - branches.emplace_back(std::move(m_stores)); - m_stores = preState; + branches.emplace_back(std::move(m_activeStores)); + m_activeStores = preState; } if (hasDefault) { - m_stores = std::move(branches.back()); + m_activeStores = std::move(branches.back()); branches.pop_back(); } for (auto& branch: branches) - merge(m_stores, std::move(branch)); + merge(m_activeStores, std::move(branch)); } void UnusedStoreBase::operator()(FunctionDefinition const& _functionDefinition) { - ScopedSaveAndRestore outerAssignments(m_stores, {}); + ScopedSaveAndRestore outerAssignments(m_activeStores, {}); ScopedSaveAndRestore forLoopInfo(m_forLoopInfo, {}); ScopedSaveAndRestore forLoopNestingDepth(m_forLoopNestingDepth, 0); @@ -94,10 +94,10 @@ void UnusedStoreBase::operator()(ForLoop const& _forLoop) visit(*_forLoop.condition); - TrackedStores zeroRuns{m_stores}; + ActiveStores zeroRuns{m_activeStores}; (*this)(_forLoop.body); - merge(m_stores, std::move(m_forLoopInfo.pendingContinueStmts)); + merge(m_activeStores, std::move(m_forLoopInfo.pendingContinueStmts)); m_forLoopInfo.pendingContinueStmts = {}; (*this)(_forLoop.post); @@ -106,54 +106,54 @@ void UnusedStoreBase::operator()(ForLoop const& _forLoop) if (m_forLoopNestingDepth < 6) { // Do the second run only for small nesting depths to avoid horrible runtime. - TrackedStores oneRun{m_stores}; + ActiveStores oneRun{m_activeStores}; (*this)(_forLoop.body); - merge(m_stores, std::move(m_forLoopInfo.pendingContinueStmts)); + merge(m_activeStores, std::move(m_forLoopInfo.pendingContinueStmts)); m_forLoopInfo.pendingContinueStmts.clear(); (*this)(_forLoop.post); visit(*_forLoop.condition); // Order of merging does not matter because "max" is commutative and associative. - merge(m_stores, std::move(oneRun)); + merge(m_activeStores, std::move(oneRun)); } else // Shortcut to avoid horrible runtime. shortcutNestedLoop(zeroRuns); // Order of merging does not matter because "max" is commutative and associative. - merge(m_stores, std::move(zeroRuns)); - merge(m_stores, std::move(m_forLoopInfo.pendingBreakStmts)); + merge(m_activeStores, std::move(zeroRuns)); + merge(m_activeStores, std::move(m_forLoopInfo.pendingBreakStmts)); m_forLoopInfo.pendingBreakStmts.clear(); } void UnusedStoreBase::operator()(Break const&) { - m_forLoopInfo.pendingBreakStmts.emplace_back(std::move(m_stores)); - m_stores.clear(); + m_forLoopInfo.pendingBreakStmts.emplace_back(std::move(m_activeStores)); + m_activeStores.clear(); } void UnusedStoreBase::operator()(Continue const&) { - m_forLoopInfo.pendingContinueStmts.emplace_back(std::move(m_stores)); - m_stores.clear(); + m_forLoopInfo.pendingContinueStmts.emplace_back(std::move(m_activeStores)); + m_activeStores.clear(); } -void UnusedStoreBase::merge(TrackedStores& _target, TrackedStores&& _other) +void UnusedStoreBase::merge(ActiveStores& _target, ActiveStores&& _other) { util::joinMap(_target, std::move(_other), []( - map& _assignmentHere, - map&& _assignmentThere + set& _storesHere, + set&& _storesThere ) { - return util::joinMap(_assignmentHere, std::move(_assignmentThere), State::join); + _storesHere += _storesThere; }); } -void UnusedStoreBase::merge(TrackedStores& _target, vector&& _source) +void UnusedStoreBase::merge(ActiveStores& _target, vector&& _source) { - for (TrackedStores& ts: _source) + for (ActiveStores& ts: _source) merge(_target, std::move(ts)); _source.clear(); } diff --git a/libyul/optimiser/UnusedStoreBase.h b/libyul/optimiser/UnusedStoreBase.h index 15dccb04a..35989da01 100644 --- a/libyul/optimiser/UnusedStoreBase.h +++ b/libyul/optimiser/UnusedStoreBase.h @@ -57,28 +57,12 @@ public: void operator()(Continue const&) override; protected: - class State - { - public: - enum Value { Unused, Undecided, Used }; - State(Value _value = Undecided): m_value(_value) {} - inline bool operator==(State _other) const { return m_value == _other.m_value; } - inline bool operator!=(State _other) const { return !operator==(_other); } - static inline void join(State& _a, State const& _b) - { - // Using "max" works here because of the order of the values in the enum. - _a.m_value = Value(std::max(int(_a.m_value), int(_b.m_value))); - } - private: - Value m_value = Undecided; - }; - - using TrackedStores = std::map>; + using ActiveStores = std::map>; /// This function is called for a loop that is nested too deep to avoid /// horrible runtime and should just resolve the situation in a pragmatic /// and correct manner. - virtual void shortcutNestedLoop(TrackedStores const& _beforeLoop) = 0; + virtual void shortcutNestedLoop(ActiveStores const& _beforeLoop) = 0; /// This function is called right before the scoped restore of the function definition. virtual void finalizeFunctionDefinition(FunctionDefinition const& /*_functionDefinition*/) {} @@ -86,20 +70,24 @@ protected: /// Joins the assignment mapping of @a _source into @a _target according to the rules laid out /// above. /// Will destroy @a _source. - static void merge(TrackedStores& _target, TrackedStores&& _source); - static void merge(TrackedStores& _target, std::vector&& _source); + static void merge(ActiveStores& _target, ActiveStores&& _source); + static void merge(ActiveStores& _target, std::vector&& _source); Dialect const& m_dialect; - std::set m_pendingRemovals; - TrackedStores m_stores; + /// 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; + /// Active (undecided) stores in the current branch. + ActiveStores m_activeStores; /// Working data for traversing for-loops. struct ForLoopInfo { /// Tracked assignment states for each break statement. - std::vector pendingBreakStmts; + std::vector pendingBreakStmts; /// Tracked assignment states for each continue statement. - std::vector pendingContinueStmts; + std::vector pendingContinueStmts; }; ForLoopInfo m_forLoopInfo; size_t m_forLoopNestingDepth = 0; diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 2e96be2e7..aca7dd907 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -50,359 +50,359 @@ 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, - CallGraphGenerator::callGraph(_ast) - ); +// map functionSideEffects = SideEffectsPropagator::sideEffects( +// _context.dialect, +// CallGraphGenerator::callGraph(_ast) +// ); - SSAValueTracker ssaValues; - ssaValues(_ast); - map values; - for (auto const& [name, expression]: ssaValues.values()) - values[name] = AssignedValue{expression, {}}; - Expression const zeroLiteral{Literal{{}, LiteralKind::Number, YulString{"0"}, {}}}; - Expression const oneLiteral{Literal{{}, LiteralKind::Number, YulString{"1"}, {}}}; - Expression const thirtyTwoLiteral{Literal{{}, LiteralKind::Number, YulString{"32"}, {}}}; - values[YulString{zero}] = AssignedValue{&zeroLiteral, {}}; - values[YulString{one}] = AssignedValue{&oneLiteral, {}}; - values[YulString{thirtyTwo}] = AssignedValue{&thirtyTwoLiteral, {}}; +// SSAValueTracker ssaValues; +// ssaValues(_ast); +// map values; +// for (auto const& [name, expression]: ssaValues.values()) +// values[name] = AssignedValue{expression, {}}; +// Expression const zeroLiteral{Literal{{}, LiteralKind::Number, YulString{"0"}, {}}}; +// Expression const oneLiteral{Literal{{}, LiteralKind::Number, YulString{"1"}, {}}}; +// Expression const thirtyTwoLiteral{Literal{{}, LiteralKind::Number, YulString{"32"}, {}}}; +// values[YulString{zero}] = AssignedValue{&zeroLiteral, {}}; +// values[YulString{one}] = AssignedValue{&oneLiteral, {}}; +// values[YulString{thirtyTwo}] = AssignedValue{&thirtyTwoLiteral, {}}; - bool const ignoreMemory = MSizeFinder::containsMSize(_context.dialect, _ast); - UnusedStoreEliminator rse{ - _context.dialect, - functionSideEffects, - ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed(), - values, - ignoreMemory - }; - rse(_ast); - if ( - auto evmDialect = dynamic_cast(&_context.dialect); - evmDialect && evmDialect->providesObjectAccess() - ) - rse.changeUndecidedTo(State::Unused, Location::Memory); - else - rse.changeUndecidedTo(State::Used, Location::Memory); - rse.changeUndecidedTo(State::Used, Location::Storage); - rse.scheduleUnusedForDeletion(); +// bool const ignoreMemory = MSizeFinder::containsMSize(_context.dialect, _ast); +// UnusedStoreEliminator rse{ +// _context.dialect, +// functionSideEffects, +// ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed(), +// values, +// ignoreMemory +// }; +// rse(_ast); +// if ( +// auto evmDialect = dynamic_cast(&_context.dialect); +// evmDialect && evmDialect->providesObjectAccess() +// ) +// rse.changeUndecidedTo(State::Unused, Location::Memory); +// else +// rse.changeUndecidedTo(State::Used, Location::Memory); +// rse.changeUndecidedTo(State::Used, Location::Storage); +// rse.scheduleUnusedForDeletion(); - StatementRemover remover(rse.m_pendingRemovals); - remover(_ast); +// StatementRemover remover(rse.m_pendingRemovals); +// remover(_ast); } -void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall) -{ - UnusedStoreBase::operator()(_functionCall); +//void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall) +//{ +// UnusedStoreBase::operator()(_functionCall); - for (Operation const& op: operationsFromFunctionCall(_functionCall)) - applyOperation(op); +// for (Operation const& op: operationsFromFunctionCall(_functionCall)) +// applyOperation(op); - ControlFlowSideEffects sideEffects; - if (auto builtin = m_dialect.builtin(_functionCall.functionName.name)) - sideEffects = builtin->controlFlowSideEffects; - else - sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name); +// ControlFlowSideEffects sideEffects; +// if (auto builtin = m_dialect.builtin(_functionCall.functionName.name)) +// sideEffects = builtin->controlFlowSideEffects; +// else +// sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name); - if (sideEffects.canTerminate) - changeUndecidedTo(State::Used, Location::Storage); - if (!sideEffects.canContinue) - { - changeUndecidedTo(State::Unused, Location::Memory); - if (!sideEffects.canTerminate) - changeUndecidedTo(State::Unused, Location::Storage); - } -} +// if (sideEffects.canTerminate) +// changeUndecidedTo(State::Used, Location::Storage); +// if (!sideEffects.canContinue) +// { +// changeUndecidedTo(State::Unused, Location::Memory); +// if (!sideEffects.canTerminate) +// changeUndecidedTo(State::Unused, Location::Storage); +// } +//} -void UnusedStoreEliminator::operator()(FunctionDefinition const& _functionDefinition) -{ - ScopedSaveAndRestore storeOperations(m_storeOperations, {}); - UnusedStoreBase::operator()(_functionDefinition); -} +//void UnusedStoreEliminator::operator()(FunctionDefinition const& _functionDefinition) +//{ +// ScopedSaveAndRestore storeOperations(m_storeOperations, {}); +// UnusedStoreBase::operator()(_functionDefinition); +//} -void UnusedStoreEliminator::operator()(Leave const&) -{ - changeUndecidedTo(State::Used); -} +//void UnusedStoreEliminator::operator()(Leave const&) +//{ +// changeUndecidedTo(State::Used); +//} -void UnusedStoreEliminator::visit(Statement const& _statement) -{ - using evmasm::Instruction; +//void UnusedStoreEliminator::visit(Statement const& _statement) +//{ +// using evmasm::Instruction; - UnusedStoreBase::visit(_statement); +// UnusedStoreBase::visit(_statement); - auto const* exprStatement = get_if(&_statement); - if (!exprStatement) - return; +// auto const* exprStatement = get_if(&_statement); +// if (!exprStatement) +// return; - FunctionCall const* funCall = get_if(&exprStatement->expression); - yulAssert(funCall); - optional instruction = toEVMInstruction(m_dialect, funCall->functionName.name); - if (!instruction) - return; +// FunctionCall const* funCall = get_if(&exprStatement->expression); +// yulAssert(funCall); +// optional instruction = toEVMInstruction(m_dialect, funCall->functionName.name); +// if (!instruction) +// return; - if (!ranges::all_of(funCall->arguments, [](Expression const& _expr) -> bool { - return get_if(&_expr) || get_if(&_expr); - })) - return; +// if (!ranges::all_of(funCall->arguments, [](Expression const& _expr) -> bool { +// return get_if(&_expr) || get_if(&_expr); +// })) +// return; - // We determine if this is a store instruction without additional side-effects - // both by querying a combination of semantic information and by listing the instructions. - // This way the assert below should be triggered on any change. - using evmasm::SemanticInformation; - bool isStorageWrite = (*instruction == Instruction::SSTORE); - bool isMemoryWrite = - *instruction == Instruction::EXTCODECOPY || - *instruction == Instruction::CODECOPY || - *instruction == Instruction::CALLDATACOPY || - *instruction == Instruction::RETURNDATACOPY || - *instruction == Instruction::MSTORE || - *instruction == Instruction::MSTORE8; - bool isCandidateForRemoval = - SemanticInformation::otherState(*instruction) != SemanticInformation::Write && ( - SemanticInformation::storage(*instruction) == SemanticInformation::Write || - (!m_ignoreMemory && SemanticInformation::memory(*instruction) == SemanticInformation::Write) - ); - yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite))); - if (isCandidateForRemoval) - { - State initialState = State::Undecided; - if (*instruction == Instruction::RETURNDATACOPY) - { - initialState = State::Used; - auto startOffset = identifierNameIfSSA(funCall->arguments.at(1)); - auto length = identifierNameIfSSA(funCall->arguments.at(2)); - KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); - if (length && startOffset) - { - FunctionCall const* lengthCall = get_if(m_ssaValues.at(*length).value); - if ( - knowledge.knownToBeZero(*startOffset) && - lengthCall && - toEVMInstruction(m_dialect, lengthCall->functionName.name) == Instruction::RETURNDATASIZE - ) - initialState = State::Undecided; - } - } - m_stores[YulString{}].insert({&_statement, initialState}); - vector operations = operationsFromFunctionCall(*funCall); - yulAssert(operations.size() == 1, ""); - m_storeOperations[&_statement] = std::move(operations.front()); - } -} +// // We determine if this is a store instruction without additional side-effects +// // both by querying a combination of semantic information and by listing the instructions. +// // This way the assert below should be triggered on any change. +// using evmasm::SemanticInformation; +// bool isStorageWrite = (*instruction == Instruction::SSTORE); +// bool isMemoryWrite = +// *instruction == Instruction::EXTCODECOPY || +// *instruction == Instruction::CODECOPY || +// *instruction == Instruction::CALLDATACOPY || +// *instruction == Instruction::RETURNDATACOPY || +// *instruction == Instruction::MSTORE || +// *instruction == Instruction::MSTORE8; +// bool isCandidateForRemoval = +// SemanticInformation::otherState(*instruction) != SemanticInformation::Write && ( +// SemanticInformation::storage(*instruction) == SemanticInformation::Write || +// (!m_ignoreMemory && SemanticInformation::memory(*instruction) == SemanticInformation::Write) +// ); +// yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite))); +// if (isCandidateForRemoval) +// { +// State initialState = State::Undecided; +// if (*instruction == Instruction::RETURNDATACOPY) +// { +// initialState = State::Used; +// auto startOffset = identifierNameIfSSA(funCall->arguments.at(1)); +// auto length = identifierNameIfSSA(funCall->arguments.at(2)); +// KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); +// if (length && startOffset) +// { +// FunctionCall const* lengthCall = get_if(m_ssaValues.at(*length).value); +// if ( +// knowledge.knownToBeZero(*startOffset) && +// lengthCall && +// toEVMInstruction(m_dialect, lengthCall->functionName.name) == Instruction::RETURNDATASIZE +// ) +// initialState = State::Undecided; +// } +// } +// m_activeStores[YulString{}].insert({&_statement, initialState}); +// vector operations = operationsFromFunctionCall(*funCall); +// yulAssert(operations.size() == 1, ""); +// m_storeOperations[&_statement] = std::move(operations.front()); +// } +//} -void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&) -{ - changeUndecidedTo(State::Used); - scheduleUnusedForDeletion(); -} +//void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&) +//{ +// changeUndecidedTo(State::Used); +// scheduleUnusedForDeletion(); +//} -vector UnusedStoreEliminator::operationsFromFunctionCall( - FunctionCall const& _functionCall -) const -{ - using evmasm::Instruction; +//vector UnusedStoreEliminator::operationsFromFunctionCall( +// FunctionCall const& _functionCall +//) const +//{ +// using evmasm::Instruction; - YulString functionName = _functionCall.functionName.name; - SideEffects sideEffects; - if (BuiltinFunction const* f = m_dialect.builtin(functionName)) - sideEffects = f->sideEffects; - else - sideEffects = m_functionSideEffects.at(functionName); +// YulString functionName = _functionCall.functionName.name; +// SideEffects sideEffects; +// if (BuiltinFunction const* f = m_dialect.builtin(functionName)) +// sideEffects = f->sideEffects; +// else +// sideEffects = m_functionSideEffects.at(functionName); - optional instruction = toEVMInstruction(m_dialect, functionName); - if (!instruction) - { - vector result; - // Unknown read is worse than unknown write. - if (sideEffects.memory != SideEffects::Effect::None) - result.emplace_back(Operation{Location::Memory, Effect::Read, {}, {}}); - if (sideEffects.storage != SideEffects::Effect::None) - result.emplace_back(Operation{Location::Storage, Effect::Read, {}, {}}); - return result; - } +// optional instruction = toEVMInstruction(m_dialect, functionName); +// if (!instruction) +// { +// vector result; +// // Unknown read is worse than unknown write. +// if (sideEffects.memory != SideEffects::Effect::None) +// result.emplace_back(Operation{Location::Memory, Effect::Read, {}, {}}); +// if (sideEffects.storage != SideEffects::Effect::None) +// result.emplace_back(Operation{Location::Storage, Effect::Read, {}, {}}); +// return result; +// } - using evmasm::SemanticInformation; +// using evmasm::SemanticInformation; - return util::applyMap( - SemanticInformation::readWriteOperations(*instruction), - [&](SemanticInformation::Operation const& _op) -> Operation - { - yulAssert(!(_op.lengthParameter && _op.lengthConstant)); - yulAssert(_op.effect != Effect::None); - Operation ourOp{_op.location, _op.effect, {}, {}}; - if (_op.startParameter) - ourOp.start = identifierNameIfSSA(_functionCall.arguments.at(*_op.startParameter)); - if (_op.lengthParameter) - ourOp.length = identifierNameIfSSA(_functionCall.arguments.at(*_op.lengthParameter)); - if (_op.lengthConstant) - switch (*_op.lengthConstant) - { - case 1: ourOp.length = YulString(one); break; - case 32: ourOp.length = YulString(thirtyTwo); break; - default: yulAssert(false); - } - return ourOp; - } - ); -} +// return util::applyMap( +// SemanticInformation::readWriteOperations(*instruction), +// [&](SemanticInformation::Operation const& _op) -> Operation +// { +// yulAssert(!(_op.lengthParameter && _op.lengthConstant)); +// yulAssert(_op.effect != Effect::None); +// Operation ourOp{_op.location, _op.effect, {}, {}}; +// if (_op.startParameter) +// ourOp.start = identifierNameIfSSA(_functionCall.arguments.at(*_op.startParameter)); +// if (_op.lengthParameter) +// ourOp.length = identifierNameIfSSA(_functionCall.arguments.at(*_op.lengthParameter)); +// if (_op.lengthConstant) +// switch (*_op.lengthConstant) +// { +// case 1: ourOp.length = YulString(one); break; +// case 32: ourOp.length = YulString(thirtyTwo); break; +// default: yulAssert(false); +// } +// return ourOp; +// } +// ); +//} -void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation const& _operation) -{ - for (auto& [statement, state]: m_stores[YulString{}]) - if (state == State::Undecided) - { - Operation const& storeOperation = m_storeOperations.at(statement); - if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) - state = State::Used; - else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) - state = State::Unused; - } -} +//void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation const& _operation) +//{ +// for (auto& [statement, state]: m_activeStores[YulString{}]) +// if (state == State::Undecided) +// { +// Operation const& storeOperation = m_storeOperations.at(statement); +// if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) +// state = State::Used; +// else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) +// state = State::Unused; +// } +//} -bool UnusedStoreEliminator::knownUnrelated( - UnusedStoreEliminator::Operation const& _op1, - UnusedStoreEliminator::Operation const& _op2 -) const -{ - KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); +//bool UnusedStoreEliminator::knownUnrelated( +// UnusedStoreEliminator::Operation const& _op1, +// UnusedStoreEliminator::Operation const& _op2 +//) const +//{ +// KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); - if (_op1.location != _op2.location) - return true; - if (_op1.location == Location::Storage) - { - if (_op1.start && _op2.start) - { - yulAssert( - _op1.length && - _op2.length && - knowledge.valueIfKnownConstant(*_op1.length) == 1 && - knowledge.valueIfKnownConstant(*_op2.length) == 1 - ); - return knowledge.knownToBeDifferent(*_op1.start, *_op2.start); - } - } - else - { - yulAssert(_op1.location == Location::Memory, ""); - if ( - (_op1.length && knowledge.knownToBeZero(*_op1.length)) || - (_op2.length && knowledge.knownToBeZero(*_op2.length)) - ) - return true; +// if (_op1.location != _op2.location) +// return true; +// if (_op1.location == Location::Storage) +// { +// if (_op1.start && _op2.start) +// { +// yulAssert( +// _op1.length && +// _op2.length && +// knowledge.valueIfKnownConstant(*_op1.length) == 1 && +// knowledge.valueIfKnownConstant(*_op2.length) == 1 +// ); +// return knowledge.knownToBeDifferent(*_op1.start, *_op2.start); +// } +// } +// else +// { +// yulAssert(_op1.location == Location::Memory, ""); +// if ( +// (_op1.length && knowledge.knownToBeZero(*_op1.length)) || +// (_op2.length && knowledge.knownToBeZero(*_op2.length)) +// ) +// return true; - if (_op1.start && _op1.length && _op2.start) - { - optional length1 = knowledge.valueIfKnownConstant(*_op1.length); - optional start1 = knowledge.valueIfKnownConstant(*_op1.start); - optional start2 = knowledge.valueIfKnownConstant(*_op2.start); - if ( - (length1 && start1 && start2) && - *start1 + *length1 >= *start1 && // no overflow - *start1 + *length1 <= *start2 - ) - return true; - } - if (_op2.start && _op2.length && _op1.start) - { - optional length2 = knowledge.valueIfKnownConstant(*_op2.length); - optional start2 = knowledge.valueIfKnownConstant(*_op2.start); - optional start1 = knowledge.valueIfKnownConstant(*_op1.start); - if ( - (length2 && start2 && start1) && - *start2 + *length2 >= *start2 && // no overflow - *start2 + *length2 <= *start1 - ) - return true; - } +// if (_op1.start && _op1.length && _op2.start) +// { +// optional length1 = knowledge.valueIfKnownConstant(*_op1.length); +// optional start1 = knowledge.valueIfKnownConstant(*_op1.start); +// optional start2 = knowledge.valueIfKnownConstant(*_op2.start); +// if ( +// (length1 && start1 && start2) && +// *start1 + *length1 >= *start1 && // no overflow +// *start1 + *length1 <= *start2 +// ) +// return true; +// } +// if (_op2.start && _op2.length && _op1.start) +// { +// optional length2 = knowledge.valueIfKnownConstant(*_op2.length); +// optional start2 = knowledge.valueIfKnownConstant(*_op2.start); +// optional start1 = knowledge.valueIfKnownConstant(*_op1.start); +// if ( +// (length2 && start2 && start1) && +// *start2 + *length2 >= *start2 && // no overflow +// *start2 + *length2 <= *start1 +// ) +// return true; +// } - if (_op1.start && _op1.length && _op2.start && _op2.length) - { - optional length1 = knowledge.valueIfKnownConstant(*_op1.length); - optional length2 = knowledge.valueIfKnownConstant(*_op2.length); - if ( - (length1 && *length1 <= 32) && - (length2 && *length2 <= 32) && - knowledge.knownToBeDifferentByAtLeast32(*_op1.start, *_op2.start) - ) - return true; - } - } +// if (_op1.start && _op1.length && _op2.start && _op2.length) +// { +// optional length1 = knowledge.valueIfKnownConstant(*_op1.length); +// optional length2 = knowledge.valueIfKnownConstant(*_op2.length); +// if ( +// (length1 && *length1 <= 32) && +// (length2 && *length2 <= 32) && +// knowledge.knownToBeDifferentByAtLeast32(*_op1.start, *_op2.start) +// ) +// return true; +// } +// } - return false; -} +// return false; +//} -bool UnusedStoreEliminator::knownCovered( - UnusedStoreEliminator::Operation const& _covered, - UnusedStoreEliminator::Operation const& _covering -) const -{ - if (_covered.location != _covering.location) - return false; - if ( - (_covered.start && _covered.start == _covering.start) && - (_covered.length && _covered.length == _covering.length) - ) - return true; - if (_covered.location == Location::Memory) - { - KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); +//bool UnusedStoreEliminator::knownCovered( +// UnusedStoreEliminator::Operation const& _covered, +// UnusedStoreEliminator::Operation const& _covering +//) const +//{ +// if (_covered.location != _covering.location) +// return false; +// if ( +// (_covered.start && _covered.start == _covering.start) && +// (_covered.length && _covered.length == _covering.length) +// ) +// return true; +// if (_covered.location == Location::Memory) +// { +// KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); - if (_covered.length && knowledge.knownToBeZero(*_covered.length)) - return true; +// if (_covered.length && knowledge.knownToBeZero(*_covered.length)) +// return true; - // Condition (i = cover_i_ng, e = cover_e_d): - // i.start <= e.start && e.start + e.length <= i.start + i.length - if (!_covered.start || !_covering.start || !_covered.length || !_covering.length) - return false; - optional coveredLength = knowledge.valueIfKnownConstant(*_covered.length); - optional coveringLength = knowledge.valueIfKnownConstant(*_covering.length); - if (knowledge.knownToBeEqual(*_covered.start, *_covering.start)) - if (coveredLength && coveringLength && *coveredLength <= *coveringLength) - return true; - optional coveredStart = knowledge.valueIfKnownConstant(*_covered.start); - optional coveringStart = knowledge.valueIfKnownConstant(*_covering.start); - if (coveredStart && coveringStart && coveredLength && coveringLength) - if ( - *coveringStart <= *coveredStart && - *coveringStart + *coveringLength >= *coveringStart && // no overflow - *coveredStart + *coveredLength >= *coveredStart && // no overflow - *coveredStart + *coveredLength <= *coveringStart + *coveringLength - ) - return true; +// // Condition (i = cover_i_ng, e = cover_e_d): +// // i.start <= e.start && e.start + e.length <= i.start + i.length +// if (!_covered.start || !_covering.start || !_covered.length || !_covering.length) +// return false; +// optional coveredLength = knowledge.valueIfKnownConstant(*_covered.length); +// optional coveringLength = knowledge.valueIfKnownConstant(*_covering.length); +// if (knowledge.knownToBeEqual(*_covered.start, *_covering.start)) +// if (coveredLength && coveringLength && *coveredLength <= *coveringLength) +// return true; +// optional coveredStart = knowledge.valueIfKnownConstant(*_covered.start); +// optional coveringStart = knowledge.valueIfKnownConstant(*_covering.start); +// if (coveredStart && coveringStart && coveredLength && coveringLength) +// if ( +// *coveringStart <= *coveredStart && +// *coveringStart + *coveringLength >= *coveringStart && // no overflow +// *coveredStart + *coveredLength >= *coveredStart && // no overflow +// *coveredStart + *coveredLength <= *coveringStart + *coveringLength +// ) +// return true; - // TODO for this we probably need a non-overflow assumption as above. - // Condition (i = cover_i_ng, e = cover_e_d): - // i.start <= e.start && e.start + e.length <= i.start + i.length - } - return false; -} +// // TODO for this we probably need a non-overflow assumption as above. +// // Condition (i = cover_i_ng, e = cover_e_d): +// // i.start <= e.start && e.start + e.length <= i.start + i.length +// } +// return false; +//} -void UnusedStoreEliminator::changeUndecidedTo( - State _newState, - optional _onlyLocation) -{ - for (auto& [statement, state]: m_stores[YulString{}]) - if ( - state == State::Undecided && - (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) - ) - state = _newState; -} +//void UnusedStoreEliminator::changeUndecidedTo( +// State _newState, +// optional _onlyLocation) +//{ +// for (auto& [statement, state]: m_activeStores[YulString{}]) +// if ( +// state == State::Undecided && +// (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) +// ) +// state = _newState; +//} -optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& _expression) const -{ - if (Identifier const* identifier = get_if(&_expression)) - if (m_ssaValues.count(identifier->name)) - return {identifier->name}; - return nullopt; -} +//optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& _expression) const +//{ +// if (Identifier const* identifier = get_if(&_expression)) +// if (m_ssaValues.count(identifier->name)) +// return {identifier->name}; +// return nullopt; +//} -void UnusedStoreEliminator::scheduleUnusedForDeletion() -{ - for (auto const& [statement, state]: m_stores[YulString{}]) - if (state == State::Unused) - m_pendingRemovals.insert(statement); -} +//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 8b5bfd7b1..40ddd73b8 100644 --- a/libyul/optimiser/UnusedStoreEliminator.h +++ b/libyul/optimiser/UnusedStoreEliminator.h @@ -64,63 +64,63 @@ 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), - m_ignoreMemory(_ignoreMemory), - m_functionSideEffects(_functionSideEffects), - m_controlFlowSideEffects(_controlFlowSideEffects), - m_ssaValues(_ssaValues) + UnusedStoreBase(_dialect)//, +// m_ignoreMemory(_ignoreMemory), +// m_functionSideEffects(_functionSideEffects), +// m_controlFlowSideEffects(_controlFlowSideEffects), +// m_ssaValues(_ssaValues) {} - using UnusedStoreBase::operator(); - void operator()(FunctionCall const& _functionCall) override; - void operator()(FunctionDefinition const&) override; - void operator()(Leave const&) override; +// using UnusedStoreBase::operator(); +// void operator()(FunctionCall const& _functionCall) override; +// void operator()(FunctionDefinition const&) override; +// void operator()(Leave const&) override; - using UnusedStoreBase::visit; - void visit(Statement const& _statement) override; +// using UnusedStoreBase::visit; +// void visit(Statement const& _statement) override; - using Location = evmasm::SemanticInformation::Location; - using Effect = evmasm::SemanticInformation::Effect; - struct Operation - { - Location location; - Effect effect; - /// Start of affected area. Unknown if not provided. - std::optional start; - /// Length of affected area, unknown if not provided. - /// Unused for storage. - std::optional length; - }; +// using Location = evmasm::SemanticInformation::Location; +// using Effect = evmasm::SemanticInformation::Effect; +// struct Operation +// { +// Location location; +// Effect effect; +// /// Start of affected area. Unknown if not provided. +// std::optional start; +// /// Length of affected area, unknown if not provided. +// /// Unused for storage. +// std::optional length; +// }; private: - void shortcutNestedLoop(TrackedStores const&) override + void shortcutNestedLoop(ActiveStores const&) override { // We might only need to do this for newly introduced stores in the loop. - changeUndecidedTo(State::Used); +// changeUndecidedTo(State::Used); } - void finalizeFunctionDefinition(FunctionDefinition const&) override; +// void finalizeFunctionDefinition(FunctionDefinition const&) override; - std::vector operationsFromFunctionCall(FunctionCall const& _functionCall) const; - void applyOperation(Operation const& _operation); - bool knownUnrelated(Operation const& _op1, Operation const& _op2) const; - bool knownCovered(Operation const& _covered, Operation const& _covering) const; +// std::vector operationsFromFunctionCall(FunctionCall const& _functionCall) const; +// void applyOperation(Operation const& _operation); +// bool knownUnrelated(Operation const& _op1, Operation const& _op2) const; +// bool knownCovered(Operation const& _covered, Operation const& _covering) const; - void changeUndecidedTo(State _newState, std::optional _onlyLocation = std::nullopt); - void scheduleUnusedForDeletion(); +// void changeUndecidedTo(State _newState, std::optional _onlyLocation = std::nullopt); +// void scheduleUnusedForDeletion(); - std::optional identifierNameIfSSA(Expression const& _expression) const; +// std::optional identifierNameIfSSA(Expression const& _expression) const; - bool const m_ignoreMemory; - std::map const& m_functionSideEffects; - std::map m_controlFlowSideEffects; - std::map const& m_ssaValues; +// bool const m_ignoreMemory; +// std::map const& m_functionSideEffects; +// std::map m_controlFlowSideEffects; +// std::map const& m_ssaValues; - std::map m_storeOperations; +// std::map m_storeOperations; }; } diff --git a/test/libyul/yulOptimizerTests/unusedAssignEliminator/for_deep_noremove.yul b/test/libyul/yulOptimizerTests/unusedAssignEliminator/for_deep_noremove.yul index 02a735f9e..c0a84c245 100644 --- a/test/libyul/yulOptimizerTests/unusedAssignEliminator/for_deep_noremove.yul +++ b/test/libyul/yulOptimizerTests/unusedAssignEliminator/for_deep_noremove.yul @@ -46,10 +46,7 @@ // for { } 1 { } // { // for { } 1 { a := 10 } -// { -// b := 12 -// b := 11 -// } +// { b := 11 } // } // } // }