diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp index 5cf3b787a..8c6591885 100644 --- a/libevmasm/Assembly.cpp +++ b/libevmasm/Assembly.cpp @@ -307,6 +307,11 @@ Assembly& Assembly::optimise(bool _enable) count = 0; copt << "Performing optimisation..."; + // This only modifies PushTags, we have to run again to actually remove code. + BlockDeduplicator dedup(m_items); + if (dedup.deduplicate()) + count++; + { ControlFlowGraph cfg(m_items); AssemblyItems optimisedItems; @@ -349,11 +354,6 @@ Assembly& Assembly::optimise(bool _enable) m_items = move(optimisedItems); count++; } - - // This only modifies PushTags, we have to run again to actually remove code. - BlockDeduplicator dedup(m_items); - if (dedup.deduplicate()) - count++; } } diff --git a/libevmasm/BlockDeduplicator.cpp b/libevmasm/BlockDeduplicator.cpp index eadbe1b40..d930ea22b 100644 --- a/libevmasm/BlockDeduplicator.cpp +++ b/libevmasm/BlockDeduplicator.cpp @@ -35,13 +35,33 @@ bool BlockDeduplicator::deduplicate() { // Compares indices based on the suffix that starts there, ignoring tags and stopping at // opcodes that stop the control flow. + + // Virtual tag that signifies "the current block" and which is used to optimise loops. + // We abort if this virtual tag actually exists. + AssemblyItem pushSelf(PushTag, u256(-4)); + if ( + std::count(m_items.cbegin(), m_items.cend(), pushSelf.tag()) || + std::count(m_items.cbegin(), m_items.cend(), pushSelf.pushTag()) + ) + return false; + function comparator = [&](size_t _i, size_t _j) { if (_i == _j) return false; - BlockIterator first(m_items.begin() + _i, m_items.end()); - BlockIterator second(m_items.begin() + _j, m_items.end()); + // To compare recursive loops, we have to already unify PushTag opcodes of the + // block's own tag. + AssemblyItem pushFirstTag(pushSelf); + AssemblyItem pushSecondTag(pushSelf); + + if (_i < m_items.size() && m_items.at(_i).type() == Tag) + pushFirstTag = m_items.at(_i).pushTag(); + if (_j < m_items.size() && m_items.at(_j).type() == Tag) + pushSecondTag = m_items.at(_j).pushTag(); + + BlockIterator first(m_items.begin() + _i, m_items.end(), &pushFirstTag, &pushSelf); + BlockIterator second(m_items.begin() + _j, m_items.end(), &pushSecondTag, &pushSelf); BlockIterator end(m_items.end(), m_items.end()); if (first != end && (*first).type() == Tag) @@ -52,27 +72,34 @@ bool BlockDeduplicator::deduplicate() return std::lexicographical_compare(first, end, second, end); }; - set> blocksSeen(comparator); - map tagReplacement; - for (size_t i = 0; i < m_items.size(); ++i) + size_t iterations = 0; + for (; ; ++iterations) { - if (m_items.at(i).type() != Tag) - continue; - auto it = blocksSeen.find(i); - if (it == blocksSeen.end()) - blocksSeen.insert(i); - else - tagReplacement[m_items.at(i).data()] = m_items.at(*it).data(); - } - - bool ret = false; - for (AssemblyItem& item: m_items) - if (item.type() == PushTag && tagReplacement.count(item.data())) + //@todo this should probably be optimized. + set> blocksSeen(comparator); + map tagReplacement; + for (size_t i = 0; i < m_items.size(); ++i) { - ret = true; - item.setData(tagReplacement.at(item.data())); + if (m_items.at(i).type() != Tag) + continue; + auto it = blocksSeen.find(i); + if (it == blocksSeen.end()) + blocksSeen.insert(i); + else + tagReplacement[m_items.at(i).data()] = m_items.at(*it).data(); } - return ret; + + bool changed = false; + for (AssemblyItem& item: m_items) + if (item.type() == PushTag && tagReplacement.count(item.data())) + { + changed = true; + item.setData(tagReplacement.at(item.data())); + } + if (!changed) + break; + } + return iterations > 0; } BlockDeduplicator::BlockIterator& BlockDeduplicator::BlockIterator::operator++() @@ -89,3 +116,11 @@ BlockDeduplicator::BlockIterator& BlockDeduplicator::BlockIterator::operator++() } return *this; } + +AssemblyItem const& BlockDeduplicator::BlockIterator::operator*() const +{ + if (replaceItem && replaceWith && *it == *replaceItem) + return *replaceWith; + else + return *it; +} diff --git a/libevmasm/BlockDeduplicator.h b/libevmasm/BlockDeduplicator.h index 8a82a1ed7..c48835fd4 100644 --- a/libevmasm/BlockDeduplicator.h +++ b/libevmasm/BlockDeduplicator.h @@ -47,19 +47,27 @@ public: bool deduplicate(); private: - /// Iterator that skips tags skips to the end if (all branches of) the control + /// Iterator that skips tags and skips to the end if (all branches of) the control /// flow does not continue to the next instruction. + /// If the arguments are supplied to the constructor, replaces items on the fly. struct BlockIterator: std::iterator { public: - BlockIterator(AssemblyItems::const_iterator _it, AssemblyItems::const_iterator _end): - it(_it), end(_end) { } + BlockIterator( + AssemblyItems::const_iterator _it, + AssemblyItems::const_iterator _end, + AssemblyItem const* _replaceItem = nullptr, + AssemblyItem const* _replaceWith = nullptr + ): + it(_it), end(_end), replaceItem(_replaceItem), replaceWith(_replaceWith) {} BlockIterator& operator++(); bool operator==(BlockIterator const& _other) const { return it == _other.it; } bool operator!=(BlockIterator const& _other) const { return it != _other.it; } - AssemblyItem const& operator*() const { return *it; } + AssemblyItem const& operator*() const; AssemblyItems::const_iterator it; AssemblyItems::const_iterator end; + AssemblyItem const* replaceItem; + AssemblyItem const* replaceWith; }; AssemblyItems& m_items; diff --git a/libsolidity/ArrayUtils.cpp b/libsolidity/ArrayUtils.cpp index 448e4da2a..397b098c4 100644 --- a/libsolidity/ArrayUtils.cpp +++ b/libsolidity/ArrayUtils.cpp @@ -364,7 +364,13 @@ void ArrayUtils::clearStorageLoop(Type const& _type) const return; } // stack: end_pos pos - eth::AssemblyItem loopStart = m_context.newTag(); + + // jump to and return from the loop to allow for duplicate code removal + eth::AssemblyItem returnTag = m_context.pushNewTag(); + m_context << eth::Instruction::SWAP2 << eth::Instruction::SWAP1; + + // stack: end_pos pos + eth::AssemblyItem loopStart = m_context.appendJumpToNew(); m_context << loopStart; // check for loop condition m_context << eth::Instruction::DUP1 << eth::Instruction::DUP3 @@ -380,7 +386,11 @@ void ArrayUtils::clearStorageLoop(Type const& _type) const m_context.appendJumpTo(loopStart); // cleanup m_context << zeroLoopEnd; - m_context << eth::Instruction::POP; + m_context << eth::Instruction::POP << eth::Instruction::SWAP1; + // "return" + m_context << eth::Instruction::JUMP; + + m_context << returnTag; solAssert(m_context.getStackHeight() == stackHeightStart - 1, ""); } diff --git a/test/libsolidity/SolidityOptimizer.cpp b/test/libsolidity/SolidityOptimizer.cpp index 744fc48ae..827d8833a 100644 --- a/test/libsolidity/SolidityOptimizer.cpp +++ b/test/libsolidity/SolidityOptimizer.cpp @@ -1004,6 +1004,38 @@ BOOST_AUTO_TEST_CASE(block_deduplicator) BOOST_CHECK_EQUAL(pushTags.size(), 2); } +BOOST_AUTO_TEST_CASE(block_deduplicator_loops) +{ + AssemblyItems input{ + u256(0), + eth::Instruction::SLOAD, + AssemblyItem(PushTag, 1), + AssemblyItem(PushTag, 2), + eth::Instruction::JUMPI, + eth::Instruction::JUMP, + AssemblyItem(Tag, 1), + u256(5), + u256(6), + eth::Instruction::SSTORE, + AssemblyItem(PushTag, 1), + eth::Instruction::JUMP, + AssemblyItem(Tag, 2), + u256(5), + u256(6), + eth::Instruction::SSTORE, + AssemblyItem(PushTag, 2), + eth::Instruction::JUMP, + }; + BlockDeduplicator dedup(input); + dedup.deduplicate(); + + set pushTags; + for (AssemblyItem const& item: input) + if (item.type() == PushTag) + pushTags.insert(item.data()); + BOOST_CHECK_EQUAL(pushTags.size(), 1); +} + BOOST_AUTO_TEST_SUITE_END() }