diff --git a/libevmasm/SemanticInformation.cpp b/libevmasm/SemanticInformation.cpp index c47634113..e6d5fc350 100644 --- a/libevmasm/SemanticInformation.cpp +++ b/libevmasm/SemanticInformation.cpp @@ -208,6 +208,27 @@ bool SemanticInformation::movable(Instruction _instruction) return true; } +bool SemanticInformation::sideEffectFree(Instruction _instruction) +{ + // These are not really functional. + assertThrow(!isDupInstruction(_instruction) && !isSwapInstruction(_instruction), AssemblyException, ""); + + InstructionInfo info = instructionInfo(_instruction); + switch (_instruction) + { + // All the instructions that merely read memory are fine + // even though they are marked "sideEffects" in Instructions.cpp + case Instruction::KECCAK256: + case Instruction::MLOAD: + return true; + default: + break; + } + if (info.sideEffects) + return false; + return true; +} + bool SemanticInformation::invalidatesMemory(Instruction _instruction) { switch (_instruction) diff --git a/libevmasm/SemanticInformation.h b/libevmasm/SemanticInformation.h index 5830cf82d..9b256b58d 100644 --- a/libevmasm/SemanticInformation.h +++ b/libevmasm/SemanticInformation.h @@ -56,6 +56,10 @@ struct SemanticInformation /// without altering the semantics. This means it cannot depend on storage or memory, /// cannot have any side-effects, but it can depend on a call-constant state of the blockchain. static bool movable(Instruction _instruction); + /// @returns true if the instruction can be removed without changing the semantics. + /// This does not mean that it has to be deterministic or retrieve information from + /// somewhere else than purely the values of its arguments. + static bool sideEffectFree(Instruction _instruction); /// @returns true if the given instruction modifies memory. static bool invalidatesMemory(Instruction _instruction); /// @returns true if the given instruction modifies storage (even indirectly). diff --git a/libyul/Dialect.h b/libyul/Dialect.h index 16f820460..17df992c3 100644 --- a/libyul/Dialect.h +++ b/libyul/Dialect.h @@ -49,6 +49,8 @@ struct BuiltinFunction /// This means the function cannot depend on storage or memory, cannot have any side-effects, /// but it can depend on state that is constant across an EVM-call. bool movable = false; + /// If true, a call to this function can be omitted without changing semantics. + bool sideEffectFree = false; /// If true, can only accept literals as arguments and they cannot be moved to variables. bool literalArguments = false; }; diff --git a/libyul/backends/evm/EVMDialect.cpp b/libyul/backends/evm/EVMDialect.cpp index 69ae4034c..e33d9e78f 100644 --- a/libyul/backends/evm/EVMDialect.cpp +++ b/libyul/backends/evm/EVMDialect.cpp @@ -42,7 +42,7 @@ EVMDialect::EVMDialect(AsmFlavour _flavour, bool _objectAccess, langutil::EVMVer if (!m_objectAccess) return; - addFunction("datasize", 1, 1, true, true, [this]( + addFunction("datasize", 1, 1, true, true, true, [this]( FunctionCall const& _call, AbstractAssembly& _assembly, std::function @@ -59,7 +59,7 @@ EVMDialect::EVMDialect(AsmFlavour _flavour, bool _objectAccess, langutil::EVMVer _assembly.appendDataSize(m_subIDs.at(dataName)); } }); - addFunction("dataoffset", 1, 1, true, true, [this]( + addFunction("dataoffset", 1, 1, true, true, true, [this]( FunctionCall const& _call, AbstractAssembly& _assembly, std::function @@ -76,7 +76,7 @@ EVMDialect::EVMDialect(AsmFlavour _flavour, bool _objectAccess, langutil::EVMVer _assembly.appendDataOffset(m_subIDs.at(dataName)); } }); - addFunction("datacopy", 3, 0, false, false, []( + addFunction("datacopy", 3, 0, false, false, false, []( FunctionCall const&, AbstractAssembly& _assembly, std::function _visitArguments @@ -132,6 +132,7 @@ void EVMDialect::addFunction( size_t _params, size_t _returns, bool _movable, + bool _sideEffectFree, bool _literalArguments, std::function)> _generateCode ) @@ -142,6 +143,7 @@ void EVMDialect::addFunction( f.parameters.resize(_params); f.returns.resize(_returns); f.movable = _movable; + f.sideEffectFree = _sideEffectFree; f.literalArguments = _literalArguments; f.generateCode = std::move(_generateCode); } diff --git a/libyul/backends/evm/EVMDialect.h b/libyul/backends/evm/EVMDialect.h index c8284a7ec..0612bee1d 100644 --- a/libyul/backends/evm/EVMDialect.h +++ b/libyul/backends/evm/EVMDialect.h @@ -75,6 +75,7 @@ protected: size_t _params, size_t _returns, bool _movable, + bool _sideEffectFree, bool _literalArguments, std::function)> _generateCode ); diff --git a/libyul/backends/wasm/WasmDialect.cpp b/libyul/backends/wasm/WasmDialect.cpp index 62195c96b..8b6ff0e73 100644 --- a/libyul/backends/wasm/WasmDialect.cpp +++ b/libyul/backends/wasm/WasmDialect.cpp @@ -71,5 +71,6 @@ void WasmDialect::addFunction(string _name, size_t _params, size_t _returns) f.parameters.resize(_params); f.returns.resize(_returns); f.movable = false; + f.sideEffectFree = false; f.literalArguments = false; } diff --git a/libyul/optimiser/Semantics.cpp b/libyul/optimiser/Semantics.cpp index 6f54f6ab0..df3d0cbb0 100644 --- a/libyul/optimiser/Semantics.cpp +++ b/libyul/optimiser/Semantics.cpp @@ -51,21 +51,30 @@ void MovableChecker::operator()(Identifier const& _identifier) void MovableChecker::operator()(FunctionalInstruction const& _instr) { + ASTWalker::operator()(_instr); + if (!eth::SemanticInformation::movable(_instr.instruction)) m_movable = false; - else - ASTWalker::operator()(_instr); + if (!eth::SemanticInformation::sideEffectFree(_instr.instruction)) + m_sideEffectFree = false; } void MovableChecker::operator()(FunctionCall const& _functionCall) { + ASTWalker::operator()(_functionCall); + if (BuiltinFunction const* f = m_dialect.builtin(_functionCall.functionName.name)) - if (f->movable) - { - ASTWalker::operator()(_functionCall); - return; - } - m_movable = false; + { + if (!f->movable) + m_movable = false; + if (!f->sideEffectFree) + m_sideEffectFree = false; + } + else + { + m_movable = false; + m_sideEffectFree = false; + } } void MovableChecker::visit(Statement const&) diff --git a/libyul/optimiser/Semantics.h b/libyul/optimiser/Semantics.h index e4b9e5406..0899fc4c0 100644 --- a/libyul/optimiser/Semantics.h +++ b/libyul/optimiser/Semantics.h @@ -46,6 +46,8 @@ public: using ASTWalker::visit; bool movable() const { return m_movable; } + bool sideEffectFree() const { return m_sideEffectFree; } + std::set const& referencedVariables() const { return m_variableReferences; } private: @@ -54,6 +56,9 @@ private: std::set m_variableReferences; /// Is the current expression movable or not. bool m_movable = true; + /// Is the current expression side-effect free, i.e. can be removed + /// without changing the semantics. + bool m_sideEffectFree = true; }; /** diff --git a/libyul/optimiser/UnusedPruner.cpp b/libyul/optimiser/UnusedPruner.cpp index 4b1295a43..858c5223b 100644 --- a/libyul/optimiser/UnusedPruner.cpp +++ b/libyul/optimiser/UnusedPruner.cpp @@ -75,7 +75,7 @@ void UnusedPruner::operator()(Block& _block) { if (!varDecl.value) statement = Block{std::move(varDecl.location), {}}; - else if (MovableChecker(m_dialect, *varDecl.value).movable()) + else if (MovableChecker(m_dialect, *varDecl.value).sideEffectFree()) { subtractReferences(ReferencesCounter::countReferences(*varDecl.value)); statement = Block{std::move(varDecl.location), {}}; @@ -93,9 +93,8 @@ void UnusedPruner::operator()(Block& _block) else if (statement.type() == typeid(ExpressionStatement)) { ExpressionStatement& exprStmt = boost::get(statement); - if (MovableChecker(m_dialect, exprStmt.expression).movable()) + if (MovableChecker(m_dialect, exprStmt.expression).sideEffectFree()) { - // pop(x) should be movable! subtractReferences(ReferencesCounter::countReferences(exprStmt.expression)); statement = Block{std::move(exprStmt.location), {}}; } diff --git a/libyul/optimiser/UnusedPruner.h b/libyul/optimiser/UnusedPruner.h index 4f81b950d..02a25efb5 100644 --- a/libyul/optimiser/UnusedPruner.h +++ b/libyul/optimiser/UnusedPruner.h @@ -32,7 +32,7 @@ struct Dialect; /** * Optimisation stage that removes unused variables and functions and also - * removes movable expression statements. + * removes side-effect-free expression statements. * * Note that this does not remove circular references. * diff --git a/test/libyul/yulOptimizerTests/fullSimplify/identity_rules_simple.yul b/test/libyul/yulOptimizerTests/fullSimplify/identity_rules_simple.yul index fc6463d2b..1e9de7755 100644 --- a/test/libyul/yulOptimizerTests/fullSimplify/identity_rules_simple.yul +++ b/test/libyul/yulOptimizerTests/fullSimplify/identity_rules_simple.yul @@ -5,8 +5,4 @@ // ==== // step: fullSimplify // ---- -// { -// let _1 := 0 -// pop(mload(_1)) -// mstore(_1, 0) -// } +// { mstore(0, 0) } diff --git a/test/libyul/yulOptimizerTests/fullSimplify/not_applied_removes_non_constant_and_not_movable.yul b/test/libyul/yulOptimizerTests/fullSimplify/not_applied_removes_non_constant_and_not_movable.yul index eeb5ba1ae..9f916198c 100644 --- a/test/libyul/yulOptimizerTests/fullSimplify/not_applied_removes_non_constant_and_not_movable.yul +++ b/test/libyul/yulOptimizerTests/fullSimplify/not_applied_removes_non_constant_and_not_movable.yul @@ -1,6 +1,6 @@ -// div is eliminated, but keccak256 has side-effects. +// div is eliminated, but create has side-effects. { - let a := div(keccak256(0, 0), 0) + let a := div(create(0, 0, 0), 0) mstore(0, a) } // ==== @@ -8,6 +8,6 @@ // ---- // { // let _1 := 0 -// pop(keccak256(_1, _1)) +// pop(create(_1, _1, _1)) // mstore(_1, 0) // } diff --git a/test/libyul/yulOptimizerTests/fullSimplify/operations.yul b/test/libyul/yulOptimizerTests/fullSimplify/operations.yul index b0f979276..811f86780 100644 --- a/test/libyul/yulOptimizerTests/fullSimplify/operations.yul +++ b/test/libyul/yulOptimizerTests/fullSimplify/operations.yul @@ -23,7 +23,6 @@ // step: fullSimplify // ---- // { -// pop(mload(0)) // mstore(1, 0) // mstore(2, 0) // mstore(3, 0) diff --git a/test/libyul/yulOptimizerTests/ssaAndBack/for_loop.yul b/test/libyul/yulOptimizerTests/ssaAndBack/for_loop.yul index c556fbd8f..df26cae3e 100644 --- a/test/libyul/yulOptimizerTests/ssaAndBack/for_loop.yul +++ b/test/libyul/yulOptimizerTests/ssaAndBack/for_loop.yul @@ -24,8 +24,6 @@ // for { } lt(mload(a), mload(b)) { a := mload(b) } // { // let b_3 := mload(a) -// pop(mload(b_3)) -// pop(mload(b_3)) // let a_6 := mload(b_3) // b := mload(a_6) // } diff --git a/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign.yul b/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign.yul index 2dd2d98b0..0617c47e4 100644 --- a/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign.yul +++ b/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign.yul @@ -10,10 +10,6 @@ // step: ssaAndBack // ---- // { -// pop(mload(0)) -// pop(mload(1)) -// pop(mload(2)) -// pop(mload(3)) // let a_5 := mload(4) // mstore(a_5, 0) // } diff --git a/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign_if.yul b/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign_if.yul index fcdb64437..73aee8516 100644 --- a/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign_if.yul +++ b/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign_if.yul @@ -13,11 +13,6 @@ // ---- // { // let a := mload(0) -// if mload(1) -// { -// pop(mload(1)) -// pop(mload(2)) -// a := mload(3) -// } +// if mload(1) { a := mload(3) } // mstore(a, 0) // } diff --git a/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign_switch.yul b/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign_switch.yul index 024254525..8265f500e 100644 --- a/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign_switch.yul +++ b/test/libyul/yulOptimizerTests/ssaAndBack/multi_assign_switch.yul @@ -19,15 +19,7 @@ // { // let a := mload(0) // switch mload(1) -// case 0 { -// pop(mload(1)) -// pop(mload(2)) -// a := mload(3) -// } -// default { -// pop(mload(4)) -// pop(mload(5)) -// a := mload(6) -// } +// case 0 { a := mload(3) } +// default { a := mload(6) } // mstore(a, 0) // } diff --git a/test/libyul/yulOptimizerTests/ssaAndBack/simple.yul b/test/libyul/yulOptimizerTests/ssaAndBack/simple.yul index dacbc4484..15e56f4d6 100644 --- a/test/libyul/yulOptimizerTests/ssaAndBack/simple.yul +++ b/test/libyul/yulOptimizerTests/ssaAndBack/simple.yul @@ -7,7 +7,6 @@ // step: ssaAndBack // ---- // { -// pop(mload(0)) // let a_2 := mload(1) // mstore(a_2, 0) // }