From 99e96c2d664612b1c6194f622526cc3189f47e69 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 13 May 2019 11:00:45 +0200 Subject: [PATCH 1/3] Refactor termination detection. --- libevmasm/SemanticInformation.cpp | 8 +++- libevmasm/SemanticInformation.h | 1 + libyul/optimiser/DeadCodeEliminator.cpp | 64 +++++-------------------- libyul/optimiser/DeadCodeEliminator.h | 2 +- libyul/optimiser/Semantics.cpp | 38 +++++++++++++++ libyul/optimiser/Semantics.h | 30 ++++++++++++ 6 files changed, 89 insertions(+), 54 deletions(-) diff --git a/libevmasm/SemanticInformation.cpp b/libevmasm/SemanticInformation.cpp index ab539e526..c47634113 100644 --- a/libevmasm/SemanticInformation.cpp +++ b/libevmasm/SemanticInformation.cpp @@ -136,7 +136,13 @@ bool SemanticInformation::terminatesControlFlow(AssemblyItem const& _item) { if (_item.type() != Operation) return false; - switch (_item.instruction()) + else + return terminatesControlFlow(_item.instruction()); +} + +bool SemanticInformation::terminatesControlFlow(Instruction _instruction) +{ + switch (_instruction) { case Instruction::RETURN: case Instruction::SELFDESTRUCT: diff --git a/libevmasm/SemanticInformation.h b/libevmasm/SemanticInformation.h index b4cb68387..5830cf82d 100644 --- a/libevmasm/SemanticInformation.h +++ b/libevmasm/SemanticInformation.h @@ -48,6 +48,7 @@ struct SemanticInformation static bool isJumpInstruction(AssemblyItem const& _item); static bool altersControlFlow(AssemblyItem const& _item); static bool terminatesControlFlow(AssemblyItem const& _item); + static bool terminatesControlFlow(Instruction _instruction); /// @returns false if the value put on the stack by _item depends on anything else than /// the information in the current block header, memory, storage or stack. static bool isDeterministic(AssemblyItem const& _item); diff --git a/libyul/optimiser/DeadCodeEliminator.cpp b/libyul/optimiser/DeadCodeEliminator.cpp index c405f2d76..b39473758 100644 --- a/libyul/optimiser/DeadCodeEliminator.cpp +++ b/libyul/optimiser/DeadCodeEliminator.cpp @@ -19,6 +19,7 @@ */ #include +#include #include #include @@ -30,42 +31,6 @@ using namespace std; using namespace dev; using namespace yul; -namespace -{ -bool isTerminating(yul::ExpressionStatement const& _exprStmnt) -{ - if (_exprStmnt.expression.type() != typeid(FunctionalInstruction)) - return false; - - auto const& funcInstr = boost::get(_exprStmnt.expression); - - return eth::SemanticInformation::terminatesControlFlow(funcInstr.instruction); -} - -/// Returns an iterator to the first terminating statement or -/// `_block.statements.end()()` when none was found -auto findFirstTerminatingStatement(Block& _block) -{ - return find_if( - _block.statements.begin(), - _block.statements.end(), - [](Statement const& _stmnt) - { - if ( - _stmnt.type() == typeid(ExpressionStatement) && - isTerminating(boost::get(_stmnt)) - ) - return true; - else if (_stmnt.type() == typeid(Break)) - return true; - else if (_stmnt.type() == typeid(Continue)) - return true; - - return false; - } - ); -} -} void DeadCodeEliminator::operator()(ForLoop& _for) { @@ -75,24 +40,19 @@ void DeadCodeEliminator::operator()(ForLoop& _for) void DeadCodeEliminator::operator()(Block& _block) { - auto& statements = _block.statements; + TerminationFinder::ControlFlow controlFlowChange; + size_t index; + tie(controlFlowChange, index) = TerminationFinder::firstUnconditionalControlFlowChange(_block.statements); - auto firstTerminatingStatment = findFirstTerminatingStatement(_block); - - if ( - firstTerminatingStatment != statements.end() && - firstTerminatingStatment + 1 != statements.end() - ) - statements.erase( - std::remove_if( - firstTerminatingStatment + 1, - statements.end(), - [] (Statement const& _s) - { - return _s.type() != typeid(yul::FunctionDefinition); - } + // Erase everything after the terminating statement that is not a function definition. + if (controlFlowChange != TerminationFinder::ControlFlow::FlowOut && index != size_t(-1)) + _block.statements.erase( + remove_if( + _block.statements.begin() + index + 1, + _block.statements.end(), + [] (Statement const& _s) { return _s.type() != typeid(yul::FunctionDefinition); } ), - statements.end() + _block.statements.end() ); ASTModifier::operator()(_block); diff --git a/libyul/optimiser/DeadCodeEliminator.h b/libyul/optimiser/DeadCodeEliminator.h index c4d60cf4c..16c8b6d99 100644 --- a/libyul/optimiser/DeadCodeEliminator.h +++ b/libyul/optimiser/DeadCodeEliminator.h @@ -41,7 +41,7 @@ namespace yul * Because variables declared in a for loop's init block have their scope extended to the loop body, * we require ForLoopInitRewriter to run before this step. * - * Prerequisite: ForLoopInitRewriter + * Prerequisite: ForLoopInitRewriter, Function Hoister, Function Grouper */ class DeadCodeEliminator: public ASTModifier { diff --git a/libyul/optimiser/Semantics.cpp b/libyul/optimiser/Semantics.cpp index 7edf97d71..6f54f6ab0 100644 --- a/libyul/optimiser/Semantics.cpp +++ b/libyul/optimiser/Semantics.cpp @@ -72,3 +72,41 @@ void MovableChecker::visit(Statement const&) { assertThrow(false, OptimizerException, "Movability for statement requested."); } + +pair TerminationFinder::firstUnconditionalControlFlowChange( + vector const& _statements +) +{ + for (size_t i = 0; i < _statements.size(); ++i) + { + ControlFlow controlFlow = controlFlowKind(_statements[i]); + if (controlFlow != ControlFlow::FlowOut) + return {controlFlow, i}; + } + return {ControlFlow::FlowOut, size_t(-1)}; +} + +TerminationFinder::ControlFlow TerminationFinder::controlFlowKind(Statement const& _statement) +{ + if ( + _statement.type() == typeid(ExpressionStatement) && + isTerminatingBuiltin(boost::get(_statement)) + ) + return ControlFlow::Terminate; + else if (_statement.type() == typeid(Break)) + return ControlFlow::Break; + else if (_statement.type() == typeid(Continue)) + return ControlFlow::Continue; + else + return ControlFlow::FlowOut; +} + +bool TerminationFinder::isTerminatingBuiltin(ExpressionStatement const& _exprStmnt) +{ + if (_exprStmnt.expression.type() != typeid(FunctionalInstruction)) + return false; + + return eth::SemanticInformation::terminatesControlFlow( + boost::get(_exprStmnt.expression).instruction + ); +} diff --git a/libyul/optimiser/Semantics.h b/libyul/optimiser/Semantics.h index a81a489f9..e4b9e5406 100644 --- a/libyul/optimiser/Semantics.h +++ b/libyul/optimiser/Semantics.h @@ -56,4 +56,34 @@ private: bool m_movable = true; }; +/** + * Helper class to find "irregular" control flow. + * This includes termination, break and continue. + */ +class TerminationFinder +{ +public: + enum class ControlFlow { FlowOut, Break, Continue, Terminate }; + + /// @returns the index of the first statement in the provided sequence + /// that is an unconditional ``break``, ``continue`` or a + /// call to a terminating builtin function. + /// If control flow can continue at the end of the list, + /// returns `FlowOut` and ``size_t(-1)``. + /// The function might return ``FlowOut`` even though control + /// flow cannot actually continue. + static std::pair firstUnconditionalControlFlowChange( + std::vector const& _statements + ); + + /// @returns the control flow type of the given statement. + /// This function could return FlowOut even if control flow never continues. + static ControlFlow controlFlowKind(Statement const& _statement); + + /// @returns true if the expression statement is a direct + /// call to a builtin terminating function like + /// ``stop``, ``revert`` or ``return``. + static bool isTerminatingBuiltin(ExpressionStatement const& _exprStmnt); +}; + } From 439a225ceec5dce16ee1778993e3958aeacbbb5d Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 9 May 2019 22:34:09 +0200 Subject: [PATCH 2/3] Simplify single-run for loops to if statements. --- Changelog.md | 1 + libyul/optimiser/ControlFlowSimplifier.cpp | 47 ++++++++++++++++++++-- libyul/optimiser/ControlFlowSimplifier.h | 10 ++++- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/Changelog.md b/Changelog.md index d182257f1..a10da4252 100644 --- a/Changelog.md +++ b/Changelog.md @@ -10,6 +10,7 @@ Compiler Features: * SMTChecker: Support ``delete``. * SMTChecker: Inline external function calls to ``this``. * Assembler: Encode the compiler version in the deployed bytecode. + * Yul Optimizer: Simplify single-run ``for`` loops to ``if`` statements. Bugfixes: diff --git a/libyul/optimiser/ControlFlowSimplifier.cpp b/libyul/optimiser/ControlFlowSimplifier.cpp index 7040891df..a3b2f4f37 100644 --- a/libyul/optimiser/ControlFlowSimplifier.cpp +++ b/libyul/optimiser/ControlFlowSimplifier.cpp @@ -115,9 +115,51 @@ void ControlFlowSimplifier::operator()(Block& _block) simplify(_block.statements); } +void ControlFlowSimplifier::visit(Statement& _st) +{ + if (_st.type() == typeid(ForLoop)) + { + ForLoop& forLoop = boost::get(_st); + yulAssert(forLoop.pre.statements.empty(), ""); + + size_t outerBreak = m_numBreakStatements; + size_t outerContinue = m_numContinueStatements; + m_numBreakStatements = 0; + m_numContinueStatements = 0; + + ASTModifier::visit(_st); + + if (!forLoop.body.statements.empty()) + { + bool isTerminating = false; + TerminationFinder::ControlFlow controlFlow = TerminationFinder::controlFlowKind(forLoop.body.statements.back()); + if (controlFlow == TerminationFinder::ControlFlow::Break) + { + isTerminating = true; + --m_numBreakStatements; + } + else if (controlFlow == TerminationFinder::ControlFlow::Terminate) + isTerminating = true; + + if (isTerminating && m_numContinueStatements == 0 && m_numBreakStatements == 0) + { + If replacement{forLoop.location, std::move(forLoop.condition), std::move(forLoop.body)}; + if (controlFlow == TerminationFinder::ControlFlow::Break) + replacement.body.statements.resize(replacement.body.statements.size() - 1); + _st = std::move(replacement); + } + } + + m_numBreakStatements = outerBreak; + m_numContinueStatements = outerContinue; + } + else + ASTModifier::visit(_st); +} + void ControlFlowSimplifier::simplify(std::vector& _statements) { - GenericFallbackReturnsVisitor const visitor( + GenericFallbackReturnsVisitor const visitor( [&](If& _ifStmt) -> OptionalStatements { if (_ifStmt.body.statements.empty()) { @@ -136,9 +178,6 @@ void ControlFlowSimplifier::simplify(std::vector& _statements) else if (_switchStmt.cases.size() == 1) return reduceSingleCaseSwitch(_switchStmt); - return {}; - }, - [&](ForLoop&) -> OptionalStatements { return {}; } ); diff --git a/libyul/optimiser/ControlFlowSimplifier.h b/libyul/optimiser/ControlFlowSimplifier.h index 4b0b83873..a2975e5a0 100644 --- a/libyul/optimiser/ControlFlowSimplifier.h +++ b/libyul/optimiser/ControlFlowSimplifier.h @@ -38,7 +38,7 @@ namespace yul * The ControlFlowSimplifier does record the presence or absence of ``break`` * and ``continue`` statements during its traversal. * - * Prerequisite: Disambiguator, ForLoopInitRewriter. + * Prerequisite: Disambiguator, FunctionHoister, ForLoopInitRewriter. * * Important: Introduces EVM opcodes and thus can only be used on EVM code for now. */ @@ -46,9 +46,17 @@ class ControlFlowSimplifier: public ASTModifier { public: using ASTModifier::operator(); + void operator()(Break&) override { ++m_numBreakStatements; } + void operator()(Continue&) override { ++m_numContinueStatements; } void operator()(Block& _block) override; + + void visit(Statement& _st) override; + private: void simplify(std::vector& _statements); + + size_t m_numBreakStatements = 0; + size_t m_numContinueStatements = 0; }; } From 246c1c939fb0fd69b171aba85c7ed1b023489bc2 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 10 May 2019 13:19:02 +0200 Subject: [PATCH 3/3] Tests. --- .../controlFlowSimplifier/terminating_for.yul | 12 ++++++++++ .../terminating_for_nested.yul | 21 ++++++++++++++++++ .../terminating_for_nested_reversed.yul | 22 +++++++++++++++++++ .../terminating_for_revert.yul | 18 +++++++++++++++ .../terminating_for_revert_plus_break.yul | 20 +++++++++++++++++ .../terminating_for_with_continue.yul | 18 +++++++++++++++ 6 files changed, 111 insertions(+) create mode 100644 test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for.yul create mode 100644 test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_nested.yul create mode 100644 test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_nested_reversed.yul create mode 100644 test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_revert.yul create mode 100644 test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_revert_plus_break.yul create mode 100644 test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_with_continue.yul diff --git a/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for.yul b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for.yul new file mode 100644 index 000000000..6f7390d53 --- /dev/null +++ b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for.yul @@ -0,0 +1,12 @@ +{ + for {} calldatasize() { mstore(1, 2) } { + mstore(4, 5) + break + } +} +// ==== +// step: controlFlowSimplifier +// ---- +// { +// if calldatasize() { mstore(4, 5) } +// } diff --git a/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_nested.yul b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_nested.yul new file mode 100644 index 000000000..2b6397269 --- /dev/null +++ b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_nested.yul @@ -0,0 +1,21 @@ +{ + for {} calldatasize() { mstore(8, 9) } { + for {} calldatasize() { mstore(1, 2) } { + mstore(4, 5) + break + } + if mload(10) { continue } + break + } +} +// ==== +// step: controlFlowSimplifier +// ---- +// { +// for { } calldatasize() { mstore(8, 9) } +// { +// if calldatasize() { mstore(4, 5) } +// if mload(10) { continue } +// break +// } +// } diff --git a/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_nested_reversed.yul b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_nested_reversed.yul new file mode 100644 index 000000000..5412a7952 --- /dev/null +++ b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_nested_reversed.yul @@ -0,0 +1,22 @@ +{ + for {} calldatasize() { mstore(8, 9) } { + for {} calldatasize() { mstore(1, 2) } { + mstore(4, 5) + continue + } + break + } +} +// ==== +// step: controlFlowSimplifier +// ---- +// { +// if calldatasize() +// { +// for { } calldatasize() { mstore(1, 2) } +// { +// mstore(4, 5) +// continue +// } +// } +// } diff --git a/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_revert.yul b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_revert.yul new file mode 100644 index 000000000..d6f7e5d18 --- /dev/null +++ b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_revert.yul @@ -0,0 +1,18 @@ +{ + for {} calldatasize() { mstore(1, 2) } { + let x := 7 + mstore(4, 5) + revert(0, x) + } +} +// ==== +// step: controlFlowSimplifier +// ---- +// { +// if calldatasize() +// { +// let x := 7 +// mstore(4, 5) +// revert(0, x) +// } +// } diff --git a/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_revert_plus_break.yul b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_revert_plus_break.yul new file mode 100644 index 000000000..7ef55bc3c --- /dev/null +++ b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_revert_plus_break.yul @@ -0,0 +1,20 @@ +{ + for {} calldatasize() { mstore(1, 2) } { + let x := 7 + mstore(4, 5) + break + revert(0, x) + } +} +// ==== +// step: controlFlowSimplifier +// ---- +// { +// for { } calldatasize() { mstore(1, 2) } +// { +// let x := 7 +// mstore(4, 5) +// break +// revert(0, x) +// } +// } diff --git a/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_with_continue.yul b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_with_continue.yul new file mode 100644 index 000000000..9d5ce5b84 --- /dev/null +++ b/test/libyul/yulOptimizerTests/controlFlowSimplifier/terminating_for_with_continue.yul @@ -0,0 +1,18 @@ +{ + for {} calldatasize() { mstore(1, 2) } { + if calldatasize() { continue } + mstore(4, 5) + break + } +} +// ==== +// step: controlFlowSimplifier +// ---- +// { +// for { } calldatasize() { mstore(1, 2) } +// { +// if calldatasize() { continue } +// mstore(4, 5) +// break +// } +// }