eof: Implement block deduplicator for EOF. Add simple unit tests.

Author: rodiazet

Diff for: libevmasm/Assembly.cpp

@@ -22,13 +22,14 @@
 
 #include <libevmasm/Assembly.h>
 
+#include <libevmasm/BlockDeduplicator.h>
+#include <libevmasm/BlockSorter.h>
 #include <libevmasm/CommonSubexpressionEliminator.h>
+#include <libevmasm/ConstantOptimiser.h>
 #include <libevmasm/ControlFlowGraph.h>
-#include <libevmasm/PeepholeOptimiser.h>
 #include <libevmasm/Inliner.h>
 #include <libevmasm/JumpdestRemover.h>
-#include <libevmasm/BlockDeduplicator.h>
-#include <libevmasm/ConstantOptimiser.h>
+#include <libevmasm/PeepholeOptimiser.h>
 
 #include <liblangutil/CharStream.h>
 #include <liblangutil/Exceptions.h>
@@ -796,6 +797,10 @@ std::map<u256, u256> const& Assembly::optimiseInternal(
 	if (m_tagReplacements)
 		return *m_tagReplacements;
 
+	if (m_eofVersion.has_value())
+		for (auto& codeSection: m_codeSections)
+			BlockSorter{codeSection.items}.sort();
+
 	// Run optimisation for sub-assemblies.
 	// TODO: verify and double-check this for EOF.
 	for (size_t subId = 0; subId < m_subs.size(); ++subId)
@@ -856,12 +861,12 @@ std::map<u256, u256> const& Assembly::optimiseInternal(
 			}
 		}
 
-		// This only modifies PushTags, we have to run again to actually remove code.
-		// TODO: implement for EOF.
-		if (_settings.runDeduplicate && !m_eofVersion.has_value())
+		// For legacy this only modifies PushTags, we have to run again to actually remove code.
+		// For EOF it modifies RJUMP and RJUMPI tags.
+		if (_settings.runDeduplicate)
 			for (auto& section: m_codeSections)
 			{
-				BlockDeduplicator deduplicator{section.items};
+				BlockDeduplicator deduplicator{section.items, m_eofVersion};
 				if (deduplicator.deduplicate())
 				{
 					for (auto const& replacement: deduplicator.replacedTags())

Diff for: libevmasm/BlockDeduplicator.cpp

@@ -41,55 +41,130 @@ bool BlockDeduplicator::deduplicate()
 
 	// 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())
-	)
+	auto const virtualTagData = u256(-4);
+	AssemblyItem pushSelf{PushTag, u256(virtualTagData)};
+
+	// There is no PushTag in EOF context but relative jumps have their destination stored in AssmblyItem data.
+	// We need to virtually replace all destinations of these r-jumps if they point to the _item Tag data.
+	AssemblyItem rjumpSelf{RelativeJump, Instruction::RJUMP, virtualTagData};
+	AssemblyItem rjumpiSelf{ConditionalRelativeJump, Instruction::RJUMPI, virtualTagData};
+
+	if (std::count(m_items.cbegin(), m_items.cend(), pushSelf.tag()))
 		return false;
 
-	std::function<bool(size_t, size_t)> comparator = [&](size_t _i, size_t _j)
+	if (!m_eofVersion.has_value())
+	{
+		if (std::count(m_items.cbegin(), m_items.cend(), pushSelf.pushTag()))
+			return false;
+	}
+	else
 	{
-		if (_i == _j)
+		if (
+			std::count(m_items.cbegin(), m_items.cend(), rjumpSelf) ||
+			std::count(m_items.cbegin(), m_items.cend(), rjumpiSelf)
+		)
 			return false;
+	}
+
+	std::function<bool(size_t, size_t)> comparator;
+
+	if (!m_eofVersion.has_value())
+	{
+		comparator = [&](size_t _i, size_t _j)
+		{
+			if (_i == _j)
+				return false;
+
+			using diff_type = BlockIterator::difference_type;
+
+			// 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() + diff_type(_i), m_items.end(), {{pushFirstTag, pushSelf}}};
+			BlockIterator second{m_items.begin() + diff_type(_j), m_items.end(), {{pushSecondTag, pushSelf}}};
+			BlockIterator end{m_items.end(), m_items.end(), {}};
+
+			if (first != end && (*first).type() == Tag)
+				++first;
+			if (second != end && (*second).type() == Tag)
+				++second;
+
+			return std::lexicographical_compare(first, end, second, end);
+		};
+	}
+	else
+	{
+		comparator = [&](size_t _i, size_t _j)
+		{
+			if (_i == _j)
+				return false;
 
-		// To compare recursive loops, we have to already unify PushTag opcodes of the
-		// block's own tag.
-		AssemblyItem pushFirstTag{pushSelf};
-		AssemblyItem pushSecondTag{pushSelf};
+			using diff_type = BlockIterator::difference_type;
 
-		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();
+			std::map<AssemblyItem const, AssemblyItem const> replacmentMapFirst;
+			std::map<AssemblyItem const, AssemblyItem const> replacmentMapSecond;
+
+			if (_i < m_items.size() && m_items.at(_i).type() == Tag)
+			{
+				replacmentMapFirst.emplace(AssemblyItem::relativeJumpTo(m_items.at(_i)), rjumpSelf);
+				replacmentMapFirst.emplace(AssemblyItem::conditionalRelativeJumpTo(m_items.at(_i)), rjumpiSelf);
+			}
+			if (_j < m_items.size() && m_items.at(_j).type() == Tag)
+			{
+				replacmentMapSecond.emplace(AssemblyItem::relativeJumpTo(m_items.at(_j)), rjumpSelf);
+				replacmentMapSecond.emplace(AssemblyItem::conditionalRelativeJumpTo(m_items.at(_j)), rjumpiSelf);
+			}
 
-		using diff_type = BlockIterator::difference_type;
-		BlockIterator first{m_items.begin() + diff_type(_i), m_items.end(), &pushFirstTag, &pushSelf};
-		BlockIterator second{m_items.begin() + diff_type(_j), m_items.end(), &pushSecondTag, &pushSelf};
-		BlockIterator end{m_items.end(), m_items.end()};
+			BlockIterator first{m_items.begin() + diff_type(_i), m_items.end(), std::move(replacmentMapFirst)};
+			BlockIterator second{m_items.begin() + diff_type(_j), m_items.end(), std::move(replacmentMapSecond)};
+			BlockIterator end{m_items.end(), m_items.end(), {}};
 
-		if (first != end && (*first).type() == Tag)
-			++first;
-		if (second != end && (*second).type() == Tag)
-			++second;
+			if (first != end && (*first).type() == Tag)
+				++first;
+			if (second != end && (*second).type() == Tag)
+				++second;
 
-		return std::lexicographical_compare(first, end, second, end);
-	};
+			return std::lexicographical_compare(first, end, second, end);
+		};
+	}
 
 	size_t iterations = 0;
 	for (; ; ++iterations)
 	{
-		//@todo this should probably be optimized.
 		std::set<size_t, std::function<bool(size_t, size_t)>> blocksSeen(comparator);
-		for (size_t i = 0; i < m_items.size(); ++i)
+		if (!m_eofVersion.has_value())
 		{
-			if (m_items.at(i).type() != Tag)
-				continue;
-			auto it = blocksSeen.find(i);
-			if (it == blocksSeen.end())
-				blocksSeen.insert(i);
-			else
-				m_replacedTags[m_items.at(i).data()] = m_items.at(*it).data();
+			//@todo this should probably be optimized.
+			for (size_t i = 0; i < m_items.size(); ++i)
+			{
+				if (m_items.at(i).type() != Tag)
+					continue;
+				auto it = blocksSeen.find(i);
+				if (it == blocksSeen.end())
+					blocksSeen.insert(i);
+				else
+					m_replacedTags[m_items.at(i).data()] = m_items.at(*it).data();
+			}
+		}
+		else
+		{
+			for (size_t i = m_items.size(); i > 0; --i)
+			{
+				if (m_items.at(i - 1).type() != Tag)
+					continue;
+				auto it = blocksSeen.find(i - 1);
+				if (it == blocksSeen.end())
+					blocksSeen.insert(i - 1);
+				else
+					m_replacedTags[m_items.at(i - 1).data()] = m_items.at(*it).data();
+			}
 		}
 
 		if (!applyTagReplacement(m_items, m_replacedTags))
@@ -131,7 +206,12 @@ BlockDeduplicator::BlockIterator& BlockDeduplicator::BlockIterator::operator++()
 {
 	if (it == end)
 		return *this;
-	if (SemanticInformation::altersControlFlow(*it) && *it != AssemblyItem{Instruction::JUMPI} && it->type() != ConditionalRelativeJump)
+	if (
+		SemanticInformation::altersControlFlow(*it) &&
+		*it != AssemblyItem{Instruction::JUMPI} &&
+		it->type() != ConditionalRelativeJump &&
+		it->type() != CallF
+	)
 		it = end;
 	else
 	{
@@ -144,8 +224,9 @@ BlockDeduplicator::BlockIterator& BlockDeduplicator::BlockIterator::operator++()
 
 AssemblyItem const& BlockDeduplicator::BlockIterator::operator*() const
 {
-	if (replaceItem && replaceWith && *it == *replaceItem)
-		return *replaceWith;
-	else
-		return *it;
+	auto const rmIt =  m_replaceMap.find(*it);
+
+	if (rmIt != m_replaceMap.end())
+		return rmIt->second;
+	return *it;
 }

Diff for: libevmasm/BlockDeduplicator.h

@@ -46,7 +46,8 @@ using AssemblyItems = std::vector<AssemblyItem>;
 class BlockDeduplicator
 {
 public:
-	explicit BlockDeduplicator(AssemblyItems& _items): m_items(_items) {}
+	explicit BlockDeduplicator(AssemblyItems& _items, std::optional<uint8_t> _eofVersion):
+		m_items(_items), m_eofVersion(_eofVersion) {}
 	/// @returns true if something was changed
 	bool deduplicate();
 	/// @returns the tags that were replaced.
@@ -77,22 +78,21 @@ class BlockDeduplicator
 		BlockIterator(
 			AssemblyItems::const_iterator _it,
 			AssemblyItems::const_iterator _end,
-			AssemblyItem const* _replaceItem = nullptr,
-			AssemblyItem const* _replaceWith = nullptr
+			std::map<AssemblyItem const, AssemblyItem const>&& replaceMap
 		):
-			it(_it), end(_end), replaceItem(_replaceItem), replaceWith(_replaceWith) {}
+			it(_it), end(_end), m_replaceMap(replaceMap) {}
 		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;
 		AssemblyItems::const_iterator it;
 		AssemblyItems::const_iterator end;
-		AssemblyItem const* replaceItem;
-		AssemblyItem const* replaceWith;
+		std::map<AssemblyItem const, AssemblyItem const> m_replaceMap;
 	};
 
 	std::map<u256, u256> m_replacedTags;
 	AssemblyItems& m_items;
+	std::optional<uint8_t> m_eofVersion;
 };
 
 }