Change to storage operations.

libevmcore/CommonSubexpressionEliminator.cpp

@@ -45,7 +45,7 @@ vector<AssemblyItem> CommonSubexpressionEliminator::getOptimizedItems()
 	// Debug info:
 	//stream(cout, initialStackContents, targetStackContents);
 
-	return CSECodeGenerator(m_expressionClasses, m_storageWrites).generateCode(
+	return CSECodeGenerator(m_expressionClasses, m_storeOperations).generateCode(
 		initialStackContents,
 		targetStackContents
 	);
@@ -172,7 +172,7 @@ void CommonSubexpressionEliminator::storeInStorage(ExpressionClasses::Id _slot,
 			storageContents.insert(storageItem);
 	m_storageContent = move(storageContents);
 	ExpressionClasses::Id id = m_expressionClasses.find(Instruction::SSTORE, {_slot, _value}, true, m_sequenceNumber);
-	m_storageWrites.insert(StorageWriteOperation(_slot, m_sequenceNumber, id));
+	m_storeOperations.push_back(StoreOperation(_slot, m_sequenceNumber, id));
 	m_storageContent[_slot] = _value;
 	// increment a second time so that we get unique sequence numbers for writes
 	m_sequenceNumber ++;
@@ -249,6 +249,16 @@ bool SemanticInformation::isSwapInstruction(AssemblyItem const& _item)
 	return Instruction::SWAP1 <= _item.instruction() && _item.instruction() <= Instruction::SWAP16;
 }
 
+CSECodeGenerator::CSECodeGenerator(
+	ExpressionClasses& _expressionClasses,
+	vector<CSECodeGenerator::StoreOperation> const& _storeOperations
+):
+	m_expressionClasses(_expressionClasses)
+{
+	for (auto const& store: _storeOperations)
+		m_storeOperations[store.slot].push_back(store);
+}
+
 AssemblyItems CSECodeGenerator::generateCode(
 	map<int, ExpressionClasses::Id> const& _initialStack,
 	map<int, ExpressionClasses::Id> const& _targetStackContents
@@ -261,35 +271,26 @@ AssemblyItems CSECodeGenerator::generateCode(
 
 	// @todo: provide information about the positions of copies of class elements
 
-	// generate the dependency graph starting from final storage writes and target stack contents
-	for (auto it = m_storageWrites.begin(); it != m_storageWrites.end();)
-	{
-		auto next = it;
-		++next;
-		if (next == m_storageWrites.end() || next->slot != it->slot)
-			// last write to that storage slot
-			addDependencies(it->expression);
-		it = next;
-	}
+	// generate the dependency graph starting from final storage and memory writes and target stack contents
+	for (auto const& p: m_storeOperations)
+		addDependencies(p.second.back().expression);
 	for (auto const& targetItem: _targetStackContents)
 	{
 		m_finalClasses.insert(targetItem.second);
 		addDependencies(targetItem.second);
 	}
 
-	// Perform all operations on storage in order, if they are needed.
-	//@todo use better data structures to optimize these loops
-	unsigned maxSequenceNumber = 1;
-	for (StorageWriteOperation const& op: m_storageWrites)
-		maxSequenceNumber = max(maxSequenceNumber, op.sequenceNumber + 1);
-	for (unsigned sequenceNumber = 1; sequenceNumber <= maxSequenceNumber; ++sequenceNumber)
-		for (auto const& depPair: m_neededBy)
-			for (ExpressionClasses::Id const& id: {depPair.first, depPair.second})
-				if (
-					m_expressionClasses.representative(id).sequenceNumber == sequenceNumber &&
-					!m_classPositions.count(id)
-				)
-					generateClassElement(id, true);
+	// store all needed sequenced expressions
+	set<pair<unsigned, ExpressionClasses::Id>> sequencedExpressions;
+	for (auto const& p: m_neededBy)
+		for (auto id: {p.first, p.second})
+			if (unsigned seqNr = m_expressionClasses.representative(id).sequenceNumber)
+				sequencedExpressions.insert(make_pair(seqNr, id));
+
+	// Perform all operations on storage and memory in order, if they are needed.
+	for (auto const& seqAndId: sequencedExpressions)
+		if (!m_classPositions.count(seqAndId.second))
+			generateClassElement(seqAndId.second, true);
 
 	// generate the target stack elements
 	for (auto const& targetItem: _targetStackContents)
@@ -340,23 +341,25 @@ void CSECodeGenerator::addDependencies(ExpressionClasses::Id _c)
 	if (expr.item->type() == Operation && expr.item->instruction() == Instruction::SLOAD)
 	{
 		// this loads an unknown value from storage and thus, in addition to its arguments, depends
-		// on all SSTORE operations to addresses where we do not know that they are different that
-		// occur before this SLOAD
+		// on all store operations to addresses where we do not know that they are different that
+		// occur before this load
 		ExpressionClasses::Id slotToLoadFrom = expr.arguments.at(0);
-		for (auto it = m_storageWrites.begin(); it != m_storageWrites.end();)
-		{
-			auto next = it;
-			++next;
-			// note that SSTORE and SLOAD never have the same sequence number
-			if (it->sequenceNumber < expr.sequenceNumber &&
-				!m_expressionClasses.knownToBeDifferent(it->slot, slotToLoadFrom) &&
-				(next == m_storageWrites.end() || next->sequenceNumber > expr.sequenceNumber)
-			)
+		for (auto const& p: m_storeOperations)
 		{
-				addDependencies(it->expression);
-				m_neededBy.insert(make_pair(it->expression, _c));
-			}
-			it = next;
+			ExpressionClasses::Id slot = p.first;
+			StoreOperations const& storeOps = p.second;
+			if (
+				m_expressionClasses.knownToBeDifferent(slot, slotToLoadFrom) ||
+				storeOps.front().sequenceNumber > expr.sequenceNumber
+			)
+				continue;
+			// note that store and load never have the same sequence number
+			ExpressionClasses::Id latestStore = storeOps.front().expression;
+			for (auto it = ++storeOps.begin(); it != storeOps.end(); ++it)
+				if (it->sequenceNumber < expr.sequenceNumber)
+					latestStore = it->expression;
+			addDependencies(latestStore);
+			m_neededBy.insert(make_pair(latestStore, _c));
 		}
 	}
 }

libevmcore/CommonSubexpressionEliminator.h

@@ -56,18 +56,13 @@ using AssemblyItems = std::vector<AssemblyItem>;
 class CommonSubexpressionEliminator
 {
 public:
-	struct StorageWriteOperation
+	struct StoreOperation
 	{
-		StorageWriteOperation(
+		StoreOperation(
 			ExpressionClasses::Id _slot,
 			unsigned _sequenceNumber,
 			ExpressionClasses::Id _expression
 		): slot(_slot), sequenceNumber(_sequenceNumber), expression(_expression) {}
-		bool operator<(StorageWriteOperation const& _other) const
-		{
-			return std::tie(slot, sequenceNumber, expression) <
-					std::tie(_other.slot, _other.sequenceNumber, _other.expression);
-		}
 		ExpressionClasses::Id slot;
 		unsigned sequenceNumber;
 		ExpressionClasses::Id expression;
@@ -114,13 +109,13 @@ private:
 	int m_stackHeight = 0;
 	/// Current stack layout, mapping stack height -> equivalence class
 	std::map<int, ExpressionClasses::Id> m_stackElements;
-	/// Current sequence number, this is incremented with each modification to storage.
+	/// Current sequence number, this is incremented with each modification to storage or memory.
 	unsigned m_sequenceNumber = 1;
 	/// Knowledge about storage content.
 	std::map<ExpressionClasses::Id, ExpressionClasses::Id> m_storageContent;
-	/// Keeps information about which storage slots were written to at which sequence number with
-	/// what SSTORE instruction.
-	std::set<StorageWriteOperation> m_storageWrites;
+	/// Keeps information about which storage or memory slots were written to at which sequence
+	/// number with what instruction.
+	std::vector<StoreOperation> m_storeOperations;
 	/// Structure containing the classes of equivalent expressions.
 	ExpressionClasses m_expressionClasses;
 };
@@ -146,15 +141,12 @@ struct SemanticInformation
 class CSECodeGenerator
 {
 public:
-	using StorageWriteOperation = CommonSubexpressionEliminator::StorageWriteOperation;
+	using StoreOperation = CommonSubexpressionEliminator::StoreOperation;
+	using StoreOperations = std::vector<StoreOperation>;
 
-	CSECodeGenerator(
-		ExpressionClasses& _expressionClasses,
-		std::set<StorageWriteOperation> const& _storageWrites
-	):
-		m_expressionClasses(_expressionClasses),
-		m_storageWrites(_storageWrites)
-	{}
+	/// Initializes the code generator with the given classes and store operations.
+	/// The store operations have to be sorted ascendingly by sequence number.
+	CSECodeGenerator(ExpressionClasses& _expressionClasses, StoreOperations const& _storeOperations);
 
 	/// @returns the assembly items generated from the given requirements
 	/// @param _initialStack current contents of the stack (up to stack height of zero)
@@ -206,9 +198,9 @@ private:
 
 	/// The actual eqivalence class items and how to compute them.
 	ExpressionClasses& m_expressionClasses;
-	/// Keeps information about which storage slots were written to at which sequence number with
-	/// what SSTORE instruction.
-	std::set<StorageWriteOperation> const& m_storageWrites;
+	/// Keeps information about which storage or memory slots were written to by which operations.
+	/// The operations are sorted ascendingly by sequence number.
+	std::map<ExpressionClasses::Id, StoreOperations> m_storeOperations;
 	/// The set of equivalence classes that should be present on the stack at the end.
 	std::set<ExpressionClasses::Id> m_finalClasses;
 };

libevmcore/ExpressionClasses.h

@@ -61,8 +61,13 @@ public:
 	/// given classes, might also create a new one.
 	/// @param _copyItem if true, copies the assembly item to an internal storage instead of just
 	/// keeping a pointer.
-	/// The @a _sequenceNumber indicates the current storage access sequence.
-	Id find(AssemblyItem const& _item, Ids const& _arguments = {}, bool _copyItem = true, unsigned _sequenceNumber = 0);
+	/// The @a _sequenceNumber indicates the current storage or memory access sequence.
+	Id find(
+		AssemblyItem const& _item,
+		Ids const& _arguments = {},
+		bool _copyItem = true,
+		unsigned _sequenceNumber = 0
+	);
 	/// @returns the canonical representative of an expression class.
 	Expression const& representative(Id _id) const { return m_representatives.at(_id); }
 	/// @returns the number of classes.