diff --git a/libyul/optimiser/CommonSubexpressionEliminator.cpp b/libyul/optimiser/CommonSubexpressionEliminator.cpp index d25beb6bd..dddbb2472 100644 --- a/libyul/optimiser/CommonSubexpressionEliminator.cpp +++ b/libyul/optimiser/CommonSubexpressionEliminator.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -37,18 +38,13 @@ using namespace solidity::util; void CommonSubexpressionEliminator::run(OptimiserStepContext& _context, Block& _ast) { - CommonSubexpressionEliminator cse{ - _context.dialect, - SideEffectsPropagator::sideEffects(_context.dialect, CallGraphGenerator::callGraph(_ast)) - }; - cse(_ast); + CommonSubexpressionEliminator{_context.dialect, _ast}(_ast); } CommonSubexpressionEliminator::CommonSubexpressionEliminator( Dialect const& _dialect, - map _functionSideEffects -): - DataFlowAnalyzer(_dialect, std::move(_functionSideEffects)) + Block const& _ast +): DataFlowAnalyzer(_dialect, _ast) { } diff --git a/libyul/optimiser/CommonSubexpressionEliminator.h b/libyul/optimiser/CommonSubexpressionEliminator.h index 7099ad181..078723cae 100644 --- a/libyul/optimiser/CommonSubexpressionEliminator.h +++ b/libyul/optimiser/CommonSubexpressionEliminator.h @@ -49,10 +49,7 @@ public: void operator()(FunctionDefinition&) override; private: - CommonSubexpressionEliminator( - Dialect const& _dialect, - std::map _functionSideEffects - ); + CommonSubexpressionEliminator(Dialect const& _dialect, Block const& _ast); protected: using ASTModifier::visit; diff --git a/libyul/optimiser/DataFlowAnalyzer.cpp b/libyul/optimiser/DataFlowAnalyzer.cpp index 65cdf31c3..5e9b33342 100644 --- a/libyul/optimiser/DataFlowAnalyzer.cpp +++ b/libyul/optimiser/DataFlowAnalyzer.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -42,14 +43,13 @@ using namespace solidity; using namespace solidity::util; using namespace solidity::yul; -DataFlowAnalyzer::DataFlowAnalyzer( - Dialect const& _dialect, - map _functionSideEffects -): +DataFlowAnalyzer::DataFlowAnalyzer(Dialect const& _dialect, Block const& _ast): m_dialect(_dialect), - m_functionSideEffects(std::move(_functionSideEffects)), m_knowledgeBase(_dialect, [this](YulString _var) { return variableValue(_var); }) { + m_functionSideEffects = SideEffectsPropagator::sideEffects(_dialect, CallGraphGenerator::callGraph(_ast)); + m_controlFlowSideEffects = ControlFlowSideEffectsCollector{_dialect, _ast}.functionSideEffectsNamed(); + if (auto const* builtin = _dialect.memoryStoreFunction(YulString{})) m_storeFunctionName[static_cast(StoreLoadLocation::Memory)] = builtin->name; if (auto const* builtin = _dialect.memoryLoadFunction(YulString{})) @@ -117,36 +117,51 @@ void DataFlowAnalyzer::operator()(VariableDeclaration& _varDecl) void DataFlowAnalyzer::operator()(If& _if) { clearKnowledgeIfInvalidated(*_if.condition); - unordered_map storage = m_state.storage; - unordered_map memory = m_state.memory; + + State preState = m_state; ASTModifier::operator()(_if); - joinKnowledge(storage, memory); - - clearValues(assignedVariableNames(_if.body)); + if (hasFlowOutControlFlow(_if.body)) + { + joinKnowledge(preState); + clearValues(assignedVariableNames(_if.body)); + } + else + m_state = move(preState); } void DataFlowAnalyzer::operator()(Switch& _switch) { clearKnowledgeIfInvalidated(*_switch.expression); visit(*_switch.expression); - set assignedVariables; + + State preState = m_state; + + optional postState; + if (!hasDefaultCase(_switch)) + postState = m_state; + + std::set assignedVariables; for (auto& _case: _switch.cases) { - unordered_map storage = m_state.storage; - unordered_map memory = m_state.memory; + m_state = preState; (*this)(_case.body); - joinKnowledge(storage, memory); - set variables = assignedVariableNames(_case.body); - assignedVariables += variables; - // This is a little too destructive, we could retain the old values. - clearValues(variables); - clearKnowledgeIfInvalidated(_case.body); + if (hasFlowOutControlFlow(_case.body)) + { + if (postState) + joinKnowledge(*postState); + assignedVariables += assignedVariableNames(_case.body); + + postState = move(m_state); + } } - for (auto& _case: _switch.cases) - clearKnowledgeIfInvalidated(_case.body); + if (postState) + m_state = move(*postState); + else + // No outflowing case. + m_state = {}; clearValues(assignedVariables); } @@ -362,30 +377,6 @@ void DataFlowAnalyzer::clearKnowledgeIfInvalidated(Expression const& _expr) m_state.memory.clear(); } -void DataFlowAnalyzer::joinKnowledge( - unordered_map const& _olderStorage, - unordered_map const& _olderMemory -) -{ - joinKnowledgeHelper(m_state.storage, _olderStorage); - joinKnowledgeHelper(m_state.memory, _olderMemory); -} - -void DataFlowAnalyzer::joinKnowledgeHelper( - std::unordered_map& _this, - std::unordered_map const& _older -) -{ - // We clear if the key does not exist in the older map or if the value is different. - // This also works for memory because _older is an "older version" - // of m_state.memory and thus any overlapping write would have cleared the keys - // that are not known to be different inside m_state.memory already. - cxx20::erase_if(_this, mapTuple([&_older](auto&& key, auto&& currentValue){ - YulString const* oldValue = util::valueOrNullptr(_older, key); - return !oldValue || *oldValue != currentValue; - })); -} - bool DataFlowAnalyzer::inScope(YulString _variableName) const { for (auto const& scope: m_variableScopes | ranges::views::reverse) @@ -430,3 +421,32 @@ std::optional DataFlowAnalyzer::isSimpleLoad( return key->name; return {}; } + +bool DataFlowAnalyzer::hasFlowOutControlFlow(Block const& _block) const +{ + return + _block.statements.empty() || + TerminationFinder{m_dialect, &m_controlFlowSideEffects}. + controlFlowKind(_block.statements.back()) == TerminationFinder::ControlFlow::FlowOut; +} + +void DataFlowAnalyzer::joinKnowledge(State const& _olderState) +{ + joinKnowledgeHelper(m_state.storage, _olderState.storage); + joinKnowledgeHelper(m_state.memory, _olderState.memory); +} + +void DataFlowAnalyzer::joinKnowledgeHelper( + std::unordered_map& _this, + std::unordered_map const& _older +) +{ + // We clear if the key does not exist in the older map or if the value is different. + // This also works for memory because _older is an "older version" + // of m_state.memory and thus any overlapping write would have cleared the keys + // that are not known to be different inside m_state.memory already. + cxx20::erase_if(_this, mapTuple([&_older](auto&& key, auto&& currentValue){ + YulString const* oldValue = valueOrNullptr(_older, key); + return !oldValue || *oldValue != currentValue; + })); +} diff --git a/libyul/optimiser/DataFlowAnalyzer.h b/libyul/optimiser/DataFlowAnalyzer.h index a8463360b..450c71149 100644 --- a/libyul/optimiser/DataFlowAnalyzer.h +++ b/libyul/optimiser/DataFlowAnalyzer.h @@ -28,6 +28,7 @@ #include #include // Needed for m_zero below. #include +#include #include #include @@ -81,14 +82,10 @@ struct AssignedValue class DataFlowAnalyzer: public ASTModifier { public: - /// @param _functionSideEffects - /// Side-effects of user-defined functions. Worst-case side-effects are assumed - /// if this is not provided or the function is not found. - /// The parameter is mostly used to determine movability of expressions. - explicit DataFlowAnalyzer( - Dialect const& _dialect, - std::map _functionSideEffects = {} - ); + /// Construct the data flow analyzer. The passed block should be the full AST + /// because side effects of user defined functions are computed from it. + /// Otherwise, worst-case side effects are assumed. + DataFlowAnalyzer(Dialect const& _dialect, Block const& _ast); using ASTModifier::operator(); void operator()(ExpressionStatement& _statement) override; @@ -129,19 +126,6 @@ protected: /// Clears knowledge about storage or memory if they may be modified inside the expression. void clearKnowledgeIfInvalidated(Expression const& _expression); - /// Joins knowledge about storage and memory with an older point in the control-flow. - /// This only works if the current state is a direct successor of the older point, - /// i.e. `_otherStorage` and `_otherMemory` cannot have additional changes. - void joinKnowledge( - std::unordered_map const& _olderStorage, - std::unordered_map const& _olderMemory - ); - - static void joinKnowledgeHelper( - std::unordered_map& _thisData, - std::unordered_map const& _olderData - ); - /// Returns true iff the variable is in scope. bool inScope(YulString _variableName) const; @@ -168,10 +152,15 @@ protected: Expression const& _expression ) const; + /// @returns true if the block is empty or the last statement has a + /// "flow out" control flow. + bool hasFlowOutControlFlow(Block const& _block) const; + Dialect const& m_dialect; /// Side-effects of user-defined functions. Worst-case side-effects are assumed /// if this is not provided or the function is not found. std::map m_functionSideEffects; + std::map m_controlFlowSideEffects; private: struct State @@ -184,6 +173,18 @@ private: std::unordered_map storage; std::unordered_map memory; }; + + /// Joins knowledge about storage and memory with an older point in the control-flow. + /// This only works if the current state is a direct successor of the older point, + /// i.e. `_otherStorage` and `_otherMemory` cannot have additional changes. + void joinKnowledge(State const& _olderState); + + static void joinKnowledgeHelper( + std::unordered_map& _thisData, + std::unordered_map const& _olderData + ); + + State m_state; protected: diff --git a/libyul/optimiser/EqualStoreEliminator.cpp b/libyul/optimiser/EqualStoreEliminator.cpp index 542d8d50b..8e9aafba3 100644 --- a/libyul/optimiser/EqualStoreEliminator.cpp +++ b/libyul/optimiser/EqualStoreEliminator.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -36,10 +37,7 @@ using namespace solidity::yul; void EqualStoreEliminator::run(OptimiserStepContext const& _context, Block& _ast) { - EqualStoreEliminator eliminator{ - _context.dialect, - SideEffectsPropagator::sideEffects(_context.dialect, CallGraphGenerator::callGraph(_ast)) - }; + EqualStoreEliminator eliminator{_context.dialect, _ast}; eliminator(_ast); StatementRemover remover{eliminator.m_pendingRemovals}; diff --git a/libyul/optimiser/EqualStoreEliminator.h b/libyul/optimiser/EqualStoreEliminator.h index 796fcc538..abc0b3dcb 100644 --- a/libyul/optimiser/EqualStoreEliminator.h +++ b/libyul/optimiser/EqualStoreEliminator.h @@ -43,11 +43,8 @@ public: static void run(OptimiserStepContext const&, Block& _ast); private: - EqualStoreEliminator( - Dialect const& _dialect, - std::map _functionSideEffects - ): - DataFlowAnalyzer(_dialect, std::move(_functionSideEffects)) + EqualStoreEliminator(Dialect const& _dialect, Block const& _ast): + DataFlowAnalyzer(_dialect, _ast) {} protected: diff --git a/libyul/optimiser/ExpressionSimplifier.cpp b/libyul/optimiser/ExpressionSimplifier.cpp index 8c3a038f1..b9e4046e4 100644 --- a/libyul/optimiser/ExpressionSimplifier.cpp +++ b/libyul/optimiser/ExpressionSimplifier.cpp @@ -23,6 +23,8 @@ #include #include +#include +#include #include using namespace std; @@ -31,7 +33,7 @@ using namespace solidity::yul; void ExpressionSimplifier::run(OptimiserStepContext& _context, Block& _ast) { - ExpressionSimplifier{_context.dialect}(_ast); + ExpressionSimplifier{_context.dialect, _ast}(_ast); } void ExpressionSimplifier::visit(Expression& _expression) diff --git a/libyul/optimiser/ExpressionSimplifier.h b/libyul/optimiser/ExpressionSimplifier.h index f9d4e8da5..712d9491d 100644 --- a/libyul/optimiser/ExpressionSimplifier.h +++ b/libyul/optimiser/ExpressionSimplifier.h @@ -51,7 +51,9 @@ public: void visit(Expression& _expression) override; private: - explicit ExpressionSimplifier(Dialect const& _dialect): DataFlowAnalyzer(_dialect) {} + ExpressionSimplifier(Dialect const& _dialect, Block const& _ast): + DataFlowAnalyzer(_dialect, _ast) + {} }; } diff --git a/libyul/optimiser/LoadResolver.cpp b/libyul/optimiser/LoadResolver.cpp index abcfbaabb..28cc7c67d 100644 --- a/libyul/optimiser/LoadResolver.cpp +++ b/libyul/optimiser/LoadResolver.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -46,7 +47,7 @@ void LoadResolver::run(OptimiserStepContext& _context, Block& _ast) bool containsMSize = MSizeFinder::containsMSize(_context.dialect, _ast); LoadResolver{ _context.dialect, - SideEffectsPropagator::sideEffects(_context.dialect, CallGraphGenerator::callGraph(_ast)), + _ast, containsMSize, _context.expectedExecutionsPerDeployment }(_ast); diff --git a/libyul/optimiser/LoadResolver.h b/libyul/optimiser/LoadResolver.h index 93d37d779..3eb084621 100644 --- a/libyul/optimiser/LoadResolver.h +++ b/libyul/optimiser/LoadResolver.h @@ -49,11 +49,11 @@ public: private: LoadResolver( Dialect const& _dialect, - std::map _functionSideEffects, + Block const& _ast, bool _containsMSize, std::optional _expectedExecutionsPerDeployment ): - DataFlowAnalyzer(_dialect, std::move(_functionSideEffects)), + DataFlowAnalyzer(_dialect, _ast), m_containsMSize(_containsMSize), m_expectedExecutionsPerDeployment(std::move(_expectedExecutionsPerDeployment)) {} diff --git a/libyul/optimiser/Rematerialiser.cpp b/libyul/optimiser/Rematerialiser.cpp index 7dff1df80..a565456da 100644 --- a/libyul/optimiser/Rematerialiser.cpp +++ b/libyul/optimiser/Rematerialiser.cpp @@ -43,7 +43,7 @@ Rematerialiser::Rematerialiser( set _varsToAlwaysRematerialize, bool _onlySelectedVariables ): - DataFlowAnalyzer(_dialect), + DataFlowAnalyzer(_dialect, _ast), m_referenceCounts(ReferencesCounter::countReferences(_ast)), m_varsToAlwaysRematerialize(std::move(_varsToAlwaysRematerialize)), m_onlySelectedVariables(_onlySelectedVariables) @@ -87,6 +87,11 @@ void Rematerialiser::visit(Expression& _e) DataFlowAnalyzer::visit(_e); } +void LiteralRematerialiser::run(OptimiserStepContext& _context, Block& _ast) +{ + LiteralRematerialiser{_context.dialect, _ast}(_ast); +} + void LiteralRematerialiser::visit(Expression& _e) { if (holds_alternative(_e)) diff --git a/libyul/optimiser/Rematerialiser.h b/libyul/optimiser/Rematerialiser.h index 592f79d1b..243418609 100644 --- a/libyul/optimiser/Rematerialiser.h +++ b/libyul/optimiser/Rematerialiser.h @@ -23,6 +23,8 @@ #include #include +#include +#include namespace solidity::yul { @@ -85,17 +87,14 @@ class LiteralRematerialiser: public DataFlowAnalyzer { public: static constexpr char const* name{"LiteralRematerialiser"}; - static void run( - OptimiserStepContext& _context, - Block& _ast - ) { LiteralRematerialiser{_context.dialect}(_ast); } + static void run(OptimiserStepContext& _context, Block& _ast); using ASTModifier::visit; void visit(Expression& _e) override; private: - LiteralRematerialiser(Dialect const& _dialect): - DataFlowAnalyzer(_dialect) + LiteralRematerialiser(Dialect const& _dialect, Block const& _ast): + DataFlowAnalyzer(_dialect, _ast) {} }; diff --git a/libyul/optimiser/StackCompressor.cpp b/libyul/optimiser/StackCompressor.cpp index b3402e4d7..748e9143f 100644 --- a/libyul/optimiser/StackCompressor.cpp +++ b/libyul/optimiser/StackCompressor.cpp @@ -56,7 +56,9 @@ namespace class RematCandidateSelector: public DataFlowAnalyzer { public: - explicit RematCandidateSelector(Dialect const& _dialect): DataFlowAnalyzer(_dialect) {} + RematCandidateSelector(Dialect const& _dialect, Block const& _ast): + DataFlowAnalyzer(_dialect, _ast) + {} /// @returns a map from function name to rematerialisation costs to a vector of variables to rematerialise /// and variables that occur in their expression. @@ -165,7 +167,7 @@ void eliminateVariables( bool _allowMSizeOptimization ) { - RematCandidateSelector selector{_dialect}; + RematCandidateSelector selector{_dialect, _ast}; selector(_ast); map>> candidates = selector.candidates(); @@ -192,7 +194,7 @@ void eliminateVariablesOptimizedCodegen( if (std::all_of(_unreachables.begin(), _unreachables.end(), [](auto const& _item) { return _item.second.empty(); })) return; - RematCandidateSelector selector{_dialect}; + RematCandidateSelector selector{_dialect, _ast}; selector(_ast); map candidates; diff --git a/test/libyul/yulOptimizerTests/fullSuite/revert_in_switch.yul b/test/libyul/yulOptimizerTests/fullSuite/revert_in_switch.yul new file mode 100644 index 000000000..536cf5553 --- /dev/null +++ b/test/libyul/yulOptimizerTests/fullSuite/revert_in_switch.yul @@ -0,0 +1,21 @@ +{ + let x := 0 + switch calldataload(0) + case 0 { + x := calldataload(99) + } + case 1 { + if 0 { revert(0, 0) } + } + sstore(0, x) +} +// ---- +// step: fullSuite +// +// { +// { +// let x := 0 +// if iszero(calldataload(x)) { x := calldataload(99) } +// sstore(0, x) +// } +// } diff --git a/test/libyul/yulOptimizerTests/loadResolver/mload_with_revert.yul b/test/libyul/yulOptimizerTests/loadResolver/mload_with_revert.yul new file mode 100644 index 000000000..ddd974c6f --- /dev/null +++ b/test/libyul/yulOptimizerTests/loadResolver/mload_with_revert.yul @@ -0,0 +1,29 @@ +{ + let b := mload(2) + mstore(0, b) + if calldataload(1) { + mstore(2, 7) + mstore(0, 7) + return(0, 0) + } + sstore(0, mload(0)) +} +// ---- +// step: loadResolver +// +// { +// { +// let _1 := 2 +// let b := mload(_1) +// let _2 := 0 +// mstore(_2, b) +// if calldataload(1) +// { +// let _5 := 7 +// mstore(_1, _5) +// mstore(_2, _5) +// return(_2, _2) +// } +// sstore(_2, b) +// } +// } diff --git a/test/libyul/yulOptimizerTests/rematerialiser/branches_switch.yul b/test/libyul/yulOptimizerTests/rematerialiser/branches_switch.yul index 442d64654..8586b6c67 100644 --- a/test/libyul/yulOptimizerTests/rematerialiser/branches_switch.yul +++ b/test/libyul/yulOptimizerTests/rematerialiser/branches_switch.yul @@ -16,7 +16,7 @@ // case 1 { b := 1 } // default { // let x := 1 -// let y := b +// let y := 2 // b := 1 // } // pop(add(1, b))