From bc127bcc6d3f91a606d9ab4eaf6b699436708d18 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 29 Nov 2022 12:15:49 +0100 Subject: [PATCH] Take non-continuing control-flow into account. --- libyul/optimiser/UnusedAssignEliminator.cpp | 23 ++++++++++++- libyul/optimiser/UnusedAssignEliminator.h | 17 ++++++++-- ...nally_assign_before_conditional_revert.yul | 33 +++++++++++++++++++ .../conditionally_assign_before_leave.yul | 22 +++++++++++++ .../conditionally_assign_before_revert.yul | 6 +--- 5 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_conditional_revert.yul create mode 100644 test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_leave.yul diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index ae92157c5..e5582c72c 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -42,7 +43,10 @@ using namespace solidity::yul; void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) { - UnusedAssignEliminator rae{_context.dialect}; + UnusedAssignEliminator rae{ + _context.dialect, + ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed() + }; rae(_ast); rae.m_storesToRemove += rae.m_allStores - rae.m_usedStores; @@ -74,10 +78,27 @@ void UnusedAssignEliminator::operator()(FunctionDefinition const& _functionDefin UnusedStoreBase::operator()(_functionDefinition); } +void UnusedAssignEliminator::operator()(FunctionCall const& _functionCall) +{ + UnusedStoreBase::operator()(_functionCall); + + ControlFlowSideEffects sideEffects; + if (auto builtin = m_dialect.builtin(_functionCall.functionName.name)) + sideEffects = builtin->controlFlowSideEffects; + else + sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name); + + if (!sideEffects.canContinue) + // We do not return from the current function, so it is OK to also + // clear the return variables. + m_activeStores.clear(); +} + void UnusedAssignEliminator::operator()(Leave const&) { for (YulString name: m_returnVariables) markUsed(name); + m_activeStores.clear(); } void UnusedAssignEliminator::operator()(Block const& _block) diff --git a/libyul/optimiser/UnusedAssignEliminator.h b/libyul/optimiser/UnusedAssignEliminator.h index afcc6f00e..800d626eb 100644 --- a/libyul/optimiser/UnusedAssignEliminator.h +++ b/libyul/optimiser/UnusedAssignEliminator.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -99,7 +100,11 @@ struct Dialect; * For switch statements that have a "default"-case, there is no control-flow * part that skips the switch. * - * At ``leave`` statements, all return variables are set to "used". + * At ``leave`` statements, all return variables are set to "used" and the set of active statements + * is cleared. + * + * If a function or builtin is called that does not continue, the set of active statements is + * cleared for all variables. * * In the second traversal, all assignments that are not marked as "used" are removed. * @@ -114,11 +119,18 @@ public: static constexpr char const* name{"UnusedAssignEliminator"}; static void run(OptimiserStepContext&, Block& _ast); - explicit UnusedAssignEliminator(Dialect const& _dialect): UnusedStoreBase(_dialect) {} + explicit UnusedAssignEliminator( + Dialect const& _dialect, + std::map _controlFlowSideEffects + ): + UnusedStoreBase(_dialect), + m_controlFlowSideEffects(_controlFlowSideEffects) + {} void operator()(Identifier const& _identifier) override; void operator()(Assignment const& _assignment) override; void operator()(FunctionDefinition const&) override; + void operator()(FunctionCall const& _functionCall) override; void operator()(Leave const&) override; void operator()(Block const& _block) override; @@ -132,6 +144,7 @@ private: void markUsed(YulString _variable); std::set m_returnVariables; + std::map m_controlFlowSideEffects; }; } diff --git a/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_conditional_revert.yul b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_conditional_revert.yul new file mode 100644 index 000000000..3eeea6f4d --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_conditional_revert.yul @@ -0,0 +1,33 @@ +{ + function g() { + if calldataload(10) { revert(0, 0) } + } + function f() { + let a := calldataload(0) + if calldataload(1) { + // this can NOT be removed + a := 2 + g() + } + sstore(0, a) + } +} +// ---- +// step: unusedAssignEliminator +// +// { +// function g() +// { +// if calldataload(10) { revert(0, 0) } +// } +// function f() +// { +// let a := calldataload(0) +// if calldataload(1) +// { +// a := 2 +// g() +// } +// sstore(0, a) +// } +// } diff --git a/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_leave.yul b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_leave.yul new file mode 100644 index 000000000..4564f5d07 --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_leave.yul @@ -0,0 +1,22 @@ +{ + function f() { + let a := calldataload(0) + if calldataload(1) { + // this can be removed + a := 2 + leave + } + sstore(0, a) + } +} +// ---- +// step: unusedAssignEliminator +// +// { +// function f() +// { +// let a := calldataload(0) +// if calldataload(1) { leave } +// sstore(0, a) +// } +// } diff --git a/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul index 88b12264e..469da01fc 100644 --- a/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul +++ b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul @@ -12,10 +12,6 @@ // // { // let a := calldataload(0) -// if calldataload(1) -// { -// a := 2 -// revert(0, 0) -// } +// if calldataload(1) { revert(0, 0) } // sstore(0, a) // }