Allow duplicate code removal for loops.

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++;
 		}
 	}
 

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<bool(size_t, size_t)> comparator = [&](size_t _i, size_t _j)
 	{
 		if (_i == _j)
 			return false;
 
-		BlockIterator first(m_items.begin() + _i, m_items.end());
+		// To compare recursive loops, we have to already unify PushTag opcodes of the
-		BlockIterator second(m_items.begin() + _j, m_items.end());
+		// 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<size_t, function<bool(size_t, size_t)>> blocksSeen(comparator);
+	size_t iterations = 0;
-	map<u256, u256> tagReplacement;
+	for (; ; ++iterations)
-	for (size_t i = 0; i < m_items.size(); ++i)
 	{
-		if (m_items.at(i).type() != Tag)
+		//@todo this should probably be optimized.
-			continue;
+		set<size_t, function<bool(size_t, size_t)>> blocksSeen(comparator);
-		auto it = blocksSeen.find(i);
+		map<u256, u256> tagReplacement;
-		if (it == blocksSeen.end())
+		for (size_t i = 0; i < m_items.size(); ++i)
-			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()))
 		{
-			ret = true;
+			if (m_items.at(i).type() != Tag)
-			item.setData(tagReplacement.at(item.data()));
+				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;
+}

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<std::forward_iterator_tag, AssemblyItem const>
 	{
 	public:
-		BlockIterator(AssemblyItems::const_iterator _it, AssemblyItems::const_iterator _end):
+		BlockIterator(
-			it(_it), end(_end) { }
+			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;