diff --git a/libyul/optimiser/NameCollector.cpp b/libyul/optimiser/NameCollector.cpp index eca509f43..c2a82d494 100644 --- a/libyul/optimiser/NameCollector.cpp +++ b/libyul/optimiser/NameCollector.cpp @@ -30,18 +30,22 @@ using namespace solidity::util; void NameCollector::operator()(VariableDeclaration const& _varDecl) { - for (auto const& var: _varDecl.variables) - m_names.emplace(var.name); + if (m_collectWhat != OnlyFunctions) + for (auto const& var: _varDecl.variables) + m_names.emplace(var.name); } -void NameCollector::operator ()(FunctionDefinition const& _funDef) +void NameCollector::operator()(FunctionDefinition const& _funDef) { - if (m_collectWhat == VariablesAndFunctions) + if (m_collectWhat != OnlyVariables) m_names.emplace(_funDef.name); - for (auto const& arg: _funDef.parameters) - m_names.emplace(arg.name); - for (auto const& ret: _funDef.returnVariables) - m_names.emplace(ret.name); + if (m_collectWhat != OnlyFunctions) + { + for (auto const& arg: _funDef.parameters) + m_names.emplace(arg.name); + for (auto const& ret: _funDef.returnVariables) + m_names.emplace(ret.name); + } ASTWalker::operator ()(_funDef); } diff --git a/libyul/optimiser/NameCollector.h b/libyul/optimiser/NameCollector.h index 8c9dc14ff..34dd5f9b3 100644 --- a/libyul/optimiser/NameCollector.h +++ b/libyul/optimiser/NameCollector.h @@ -35,7 +35,7 @@ namespace solidity::yul class NameCollector: public ASTWalker { public: - enum CollectWhat { VariablesAndFunctions, OnlyVariables }; + enum CollectWhat { VariablesAndFunctions, OnlyVariables, OnlyFunctions }; explicit NameCollector( Block const& _block, diff --git a/libyul/optimiser/Rematerialiser.cpp b/libyul/optimiser/Rematerialiser.cpp index 6e00fcd8a..7dff1df80 100644 --- a/libyul/optimiser/Rematerialiser.cpp +++ b/libyul/optimiser/Rematerialiser.cpp @@ -37,16 +37,6 @@ void Rematerialiser::run(Dialect const& _dialect, Block& _ast, set _v Rematerialiser{_dialect, _ast, std::move(_varsToAlwaysRematerialize), _onlySelectedVariables}(_ast); } -void Rematerialiser::run( - Dialect const& _dialect, - FunctionDefinition& _function, - set _varsToAlwaysRematerialize, - bool _onlySelectedVariables -) -{ - Rematerialiser{_dialect, _function, std::move(_varsToAlwaysRematerialize), _onlySelectedVariables}(_function); -} - Rematerialiser::Rematerialiser( Dialect const& _dialect, Block& _ast, @@ -60,19 +50,6 @@ Rematerialiser::Rematerialiser( { } -Rematerialiser::Rematerialiser( - Dialect const& _dialect, - FunctionDefinition& _function, - set _varsToAlwaysRematerialize, - bool _onlySelectedVariables -): - DataFlowAnalyzer(_dialect), - m_referenceCounts(ReferencesCounter::countReferences(_function)), - m_varsToAlwaysRematerialize(std::move(_varsToAlwaysRematerialize)), - m_onlySelectedVariables(_onlySelectedVariables) -{ -} - void Rematerialiser::visit(Expression& _e) { if (holds_alternative(_e)) diff --git a/libyul/optimiser/Rematerialiser.h b/libyul/optimiser/Rematerialiser.h index d1ebb1595..592f79d1b 100644 --- a/libyul/optimiser/Rematerialiser.h +++ b/libyul/optimiser/Rematerialiser.h @@ -53,12 +53,6 @@ public: std::set _varsToAlwaysRematerialize = {}, bool _onlySelectedVariables = false ); - static void run( - Dialect const& _dialect, - FunctionDefinition& _function, - std::set _varsToAlwaysRematerialize = {}, - bool _onlySelectedVariables = false - ); protected: Rematerialiser( @@ -67,12 +61,6 @@ protected: std::set _varsToAlwaysRematerialize = {}, bool _onlySelectedVariables = false ); - Rematerialiser( - Dialect const& _dialect, - FunctionDefinition& _function, - std::set _varsToAlwaysRematerialize = {}, - bool _onlySelectedVariables = false - ); using DataFlowAnalyzer::operator(); diff --git a/libyul/optimiser/StackCompressor.cpp b/libyul/optimiser/StackCompressor.cpp index 8b06d6625..eb0629258 100644 --- a/libyul/optimiser/StackCompressor.cpp +++ b/libyul/optimiser/StackCompressor.cpp @@ -50,31 +50,41 @@ namespace /** * Class that discovers all variables that can be fully eliminated by rematerialization, * and the corresponding approximate costs. + * + * Prerequisite: Disambiguator, Function Grouper */ class RematCandidateSelector: public DataFlowAnalyzer { public: explicit RematCandidateSelector(Dialect const& _dialect): DataFlowAnalyzer(_dialect) {} - /// @returns a map from rematerialisation costs to a vector of variables to rematerialise + /// @returns a map from function name to rematerialisation costs to a vector of variables to rematerialise /// and variables that occur in their expression. /// While the map is sorted by cost, the contained vectors are sorted by the order of occurrence. - map>>> candidates() + map>>>> candidates() { - map>>> cand; - for (auto const& candidate: m_candidates) + map>>>> cand; + for (auto const& [functionName, candidate]: m_candidates) { if (size_t const* cost = util::valueOrNullptr(m_expressionCodeCost, candidate)) { size_t numRef = m_numReferences[candidate]; set const* ref = references(candidate); - cand[*cost * numRef].emplace_back(candidate, ref ? move(*ref) : set{}); + cand[functionName][*cost * numRef].emplace_back(candidate, ref ? move(*ref) : set{}); } } return cand; } using DataFlowAnalyzer::operator(); + void operator()(FunctionDefinition& _function) override + { + yulAssert(m_currentFunctionName.empty()); + m_currentFunctionName = _function.name; + DataFlowAnalyzer::operator()(_function); + m_currentFunctionName = {}; + } + void operator()(VariableDeclaration& _varDecl) override { DataFlowAnalyzer::operator()(_varDecl); @@ -84,7 +94,7 @@ public: if (AssignedValue const* value = variableValue(varName)) { yulAssert(!m_expressionCodeCost.count(varName), ""); - m_candidates.emplace_back(varName); + m_candidates.emplace_back(m_currentFunctionName, varName); m_expressionCodeCost[varName] = CodeCost::codeCost(m_dialect, *value->value); } } @@ -122,8 +132,10 @@ public: m_expressionCodeCost.erase(_variable); } - /// All candidate variables in order of occurrence. - vector m_candidates; + YulString m_currentFunctionName = {}; + + /// All candidate variables by function name, in order of occurrence. + vector> m_candidates; /// Candidate variables and the code cost of their value. map m_expressionCodeCost; /// Number of references to each candidate variable. @@ -156,62 +168,80 @@ set chooseVarsToEliminate( return varsToEliminate; } -template void eliminateVariables( Dialect const& _dialect, - ASTNode& _node, - size_t _numVariables, + Block& _ast, + map const& _numVariables, bool _allowMSizeOptimization ) { RematCandidateSelector selector{_dialect}; - selector(_node); - Rematerialiser::run(_dialect, _node, chooseVarsToEliminate(selector.candidates(), _numVariables)); - UnusedPruner::runUntilStabilised(_dialect, _node, _allowMSizeOptimization); + selector(_ast); + map>>>> candidates = selector.candidates(); + + set varsToEliminate; + for (auto const& [functionName, numVariables]: _numVariables) + { + yulAssert(numVariables > 0); + varsToEliminate += chooseVarsToEliminate(candidates[functionName], static_cast(numVariables)); + } + + Rematerialiser::run(_dialect, _ast, move(varsToEliminate)); + // Do not remove functions. + set allFunctions = NameCollector{_ast, NameCollector::OnlyFunctions}.names(); + UnusedPruner::runUntilStabilised(_dialect, _ast, _allowMSizeOptimization, nullptr, allFunctions); } -void eliminateVariables( +void eliminateVariablesOptimizedCodegen( Dialect const& _dialect, - Block& _block, - vector const& _unreachables, + Block& _ast, + map> const& _unreachables, bool _allowMSizeOptimization ) { + if (std::all_of(_unreachables.begin(), _unreachables.end(), [](auto const& _item) { return _item.second.empty(); })) + return; + RematCandidateSelector selector{_dialect}; - selector(_block); - std::map candidates; - for (auto [cost, candidatesWithCost]: selector.candidates()) - for (auto candidate: candidatesWithCost) - candidates[get<0>(candidate)] = cost; + selector(_ast); + + map candidates; + for (auto const& [functionName, candidatesInFunction]: selector.candidates()) + for (auto [cost, candidatesWithCost]: candidatesInFunction) + for (auto candidate: candidatesWithCost) + candidates[get<0>(candidate)] = cost; set varsToEliminate; // TODO: this currently ignores the fact that variables may reference other variables we want to eliminate. - for (auto const& unreachable: _unreachables) - { - map> suitableCandidates; - size_t neededSlots = unreachable.deficit; - for (auto varName: unreachable.variableChoices) + for (auto const& [functionName, unreachables]: _unreachables) + for (auto const& unreachable: unreachables) { - if (varsToEliminate.count(varName)) - --neededSlots; - else if (size_t* cost = util::valueOrNullptr(candidates, varName)) - if (!util::contains(suitableCandidates[*cost], varName)) - suitableCandidates[*cost].emplace_back(varName); - } - for (auto candidatesByCost: suitableCandidates) - { - for (auto candidate: candidatesByCost.second) - if (neededSlots--) - varsToEliminate.emplace(candidate); - else + map> suitableCandidates; + size_t neededSlots = unreachable.deficit; + for (auto varName: unreachable.variableChoices) + { + if (varsToEliminate.count(varName)) + --neededSlots; + else if (size_t* cost = util::valueOrNullptr(candidates, varName)) + if (!util::contains(suitableCandidates[*cost], varName)) + suitableCandidates[*cost].emplace_back(varName); + } + for (auto candidatesByCost: suitableCandidates) + { + for (auto candidate: candidatesByCost.second) + if (neededSlots--) + varsToEliminate.emplace(candidate); + else + break; + if (!neededSlots) break; - if (!neededSlots) - break; + } } - } - Rematerialiser::run(_dialect, _block, std::move(varsToEliminate), true); - UnusedPruner::runUntilStabilised(_dialect, _block, _allowMSizeOptimization); + Rematerialiser::run(_dialect, _ast, std::move(varsToEliminate), true); + // Do not remove functions. + set allFunctions = NameCollector{_ast, NameCollector::OnlyFunctions}.names(); + UnusedPruner::runUntilStabilised(_dialect, _ast, _allowMSizeOptimization, nullptr, allFunctions); } } @@ -239,21 +269,12 @@ bool StackCompressor::run( { yul::AsmAnalysisInfo analysisInfo = yul::AsmAnalyzer::analyzeStrictAssertCorrect(_dialect, _object); unique_ptr cfg = ControlFlowGraphBuilder::build(analysisInfo, _dialect, *_object.code); - Block& mainBlock = std::get(_object.code->statements.at(0)); - if ( - auto stackTooDeepErrors = StackLayoutGenerator::reportStackTooDeep(*cfg, YulString{}); - !stackTooDeepErrors.empty() - ) - eliminateVariables(_dialect, mainBlock, stackTooDeepErrors, allowMSizeOptimzation); - for (size_t i = 1; i < _object.code->statements.size(); ++i) - { - auto& fun = std::get(_object.code->statements[i]); - if ( - auto stackTooDeepErrors = StackLayoutGenerator::reportStackTooDeep(*cfg, fun.name); - !stackTooDeepErrors.empty() - ) - eliminateVariables(_dialect, fun.body, stackTooDeepErrors, allowMSizeOptimzation); - } + eliminateVariablesOptimizedCodegen( + _dialect, + *_object.code, + StackLayoutGenerator::reportStackTooDeep(*cfg), + allowMSizeOptimzation + ); } else for (size_t iterations = 0; iterations < _maxIterations; iterations++) @@ -261,32 +282,12 @@ bool StackCompressor::run( map stackSurplus = CompilabilityChecker(_dialect, _object, _optimizeStackAllocation).stackDeficit; if (stackSurplus.empty()) return true; - - if (stackSurplus.count(YulString{})) - { - yulAssert(stackSurplus.at({}) > 0, "Invalid surplus value."); - eliminateVariables( - _dialect, - std::get(_object.code->statements.at(0)), - static_cast(stackSurplus.at({})), - allowMSizeOptimzation - ); - } - - for (size_t i = 1; i < _object.code->statements.size(); ++i) - { - auto& fun = std::get(_object.code->statements[i]); - if (!stackSurplus.count(fun.name)) - continue; - - yulAssert(stackSurplus.at(fun.name) > 0, "Invalid surplus value."); - eliminateVariables( - _dialect, - fun, - static_cast(stackSurplus.at(fun.name)), - allowMSizeOptimzation - ); - } + eliminateVariables( + _dialect, + *_object.code, + stackSurplus, + allowMSizeOptimzation + ); } return false; } diff --git a/test/libyul/yulOptimizerTests/stackCompressor/inlineInFunction.yul b/test/libyul/yulOptimizerTests/stackCompressor/inlineInFunction.yul index 35dfa5343..a51c39f4f 100644 --- a/test/libyul/yulOptimizerTests/stackCompressor/inlineInFunction.yul +++ b/test/libyul/yulOptimizerTests/stackCompressor/inlineInFunction.yul @@ -11,7 +11,6 @@ // step: stackCompressor // // { -// { let x := 8 } // function f() // { // mstore(calldataload(calldataload(9)), add(add(add(add(add(add(add(add(add(add(add(add(add(add(add(add(add(add(calldataload(calldataload(9)), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1))