From 7c94e5462abb327016520b896178de41bea473c2 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 13 Oct 2017 18:59:04 +0200 Subject: [PATCH 1/2] Assume peephole optimizer was successful if number of pops increased. --- Changelog.md | 1 + libevmasm/Assembly.cpp | 2 +- libevmasm/PeepholeOptimiser.cpp | 11 +++++++++-- test/libevmasm/Optimiser.cpp | 14 ++++++++++++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Changelog.md b/Changelog.md index d78dbbba8..22b5c0027 100644 --- a/Changelog.md +++ b/Changelog.md @@ -13,6 +13,7 @@ Features: * Type Checker: Require ``storage`` or ``memory`` keyword for local variables as experimental 0.5.0 feature. Bugfixes: + * Optimizer: Remove unused stack computation results. * Parser: Fix source location of VariableDeclarationStatement. * Type Checker: Properly check array length and don't rely on an assertion in code generation. * Type Checker: Properly support overwriting members inherited from ``address`` in a contract diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp index df691e7dd..37c4fb22d 100644 --- a/libevmasm/Assembly.cpp +++ b/libevmasm/Assembly.cpp @@ -407,7 +407,7 @@ map Assembly::optimiseInternal( if (_settings.runPeephole) { PeepholeOptimiser peepOpt(m_items); - while (peepOpt.optimise()) + while (peepOpt.optimise() && count < 0xffffff) count++; } diff --git a/libevmasm/PeepholeOptimiser.cpp b/libevmasm/PeepholeOptimiser.cpp index 31fdd3176..168d11090 100644 --- a/libevmasm/PeepholeOptimiser.cpp +++ b/libevmasm/PeepholeOptimiser.cpp @@ -249,6 +249,11 @@ void applyMethods(OptimiserState& _state, Method, OtherMethods... _other) applyMethods(_state, _other...); } +size_t numberOfPops(AssemblyItems const& _items) +{ + return std::count(_items.begin(), _items.end(), Instruction::POP); +} + } bool PeepholeOptimiser::optimise() @@ -257,8 +262,10 @@ bool PeepholeOptimiser::optimise() while (state.i < m_items.size()) applyMethods(state, PushPop(), OpPop(), DoublePush(), DoubleSwap(), JumpToNext(), UnreachableCode(), TagConjunctions(), Identity()); if (m_optimisedItems.size() < m_items.size() || ( - m_optimisedItems.size() == m_items.size() && - eth::bytesRequired(m_optimisedItems, 3) < eth::bytesRequired(m_items, 3) + m_optimisedItems.size() == m_items.size() && ( + eth::bytesRequired(m_optimisedItems, 3) < eth::bytesRequired(m_items, 3) || + numberOfPops(m_optimisedItems) > numberOfPops(m_items) + ) )) { m_items = std::move(m_optimisedItems); diff --git a/test/libevmasm/Optimiser.cpp b/test/libevmasm/Optimiser.cpp index 9dc495810..0ab95b080 100644 --- a/test/libevmasm/Optimiser.cpp +++ b/test/libevmasm/Optimiser.cpp @@ -841,6 +841,20 @@ BOOST_AUTO_TEST_CASE(peephole_double_push) ); } +BOOST_AUTO_TEST_CASE(peephole_pop_calldatasize) +{ + AssemblyItems items{ + u256(4), + Instruction::CALLDATASIZE, + Instruction::LT, + Instruction::POP + }; + PeepholeOptimiser peepOpt(items); + for (size_t i = 0; i < 3; i++) + BOOST_CHECK(peepOpt.optimise()); + BOOST_CHECK(items.empty()); +} + BOOST_AUTO_TEST_CASE(jumpdest_removal) { AssemblyItems items{ From f5e91e4a94b01daf2bbde637abef1796f063c348 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 16 Oct 2017 18:45:21 +0200 Subject: [PATCH 2/2] Throw on too many peephole optimizer iterations. --- libevmasm/Assembly.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp index 37c4fb22d..5fab24e19 100644 --- a/libevmasm/Assembly.cpp +++ b/libevmasm/Assembly.cpp @@ -407,8 +407,11 @@ map Assembly::optimiseInternal( if (_settings.runPeephole) { PeepholeOptimiser peepOpt(m_items); - while (peepOpt.optimise() && count < 0xffffff) + while (peepOpt.optimise()) + { count++; + assertThrow(count < 64000, OptimizerException, "Peephole optimizer seems to be stuck."); + } } // This only modifies PushTags, we have to run again to actually remove code.