Common subexpression elimination ready for using pre-known state.

libevmasm/CommonSubexpressionEliminator.cpp

@@ -40,9 +40,8 @@ vector<AssemblyItem> CommonSubexpressionEliminator::getOptimizedItems()
 	int minHeight = m_state.stackHeight() + 1;
 	if (!m_state.stackElements().empty())
 		minHeight = min(minHeight, m_state.stackElements().begin()->first);
-	for (int height = minHeight; height <= 0; ++height)
+	for (int height = minHeight; height <= m_initialState.stackHeight(); ++height)
-		//@todo this is not nice as it is here - should be "unknownStackElement" - but is it really unknown?
+		initialStackContents[height] = m_initialState.stackElement(height, SourceLocation());
-		initialStackContents[height] = m_state.initialStackElement(height, SourceLocation());
 	for (int height = minHeight; height <= m_state.stackHeight(); ++height)
 		targetStackContents[height] = m_state.stackElement(height, SourceLocation());
 
@@ -50,6 +49,7 @@ vector<AssemblyItem> CommonSubexpressionEliminator::getOptimizedItems()
 	//stream(cout, initialStackContents, targetStackContents);
 
 	AssemblyItems items = CSECodeGenerator(m_state.expressionClasses(), m_storeOperations).generateCode(
+		m_initialState.stackHeight(),
 		initialStackContents,
 		targetStackContents
 	);
@@ -106,10 +106,12 @@ CSECodeGenerator::CSECodeGenerator(
 }
 
 AssemblyItems CSECodeGenerator::generateCode(
+	int _initialStackHeight,
 	map<int, Id> const& _initialStack,
 	map<int, Id> const& _targetStackContents
 )
 {
+	m_stackHeight = _initialStackHeight;
 	m_stack = _initialStack;
 	for (auto const& item: m_stack)
 		if (!m_classPositions.count(item.second))

libevmasm/CommonSubexpressionEliminator.h

@@ -61,7 +61,7 @@ public:
 	using Id = ExpressionClasses::Id;
 	using StoreOperation = KnownState::StoreOperation;
 
-	CommonSubexpressionEliminator(KnownState const& _state): m_state(_state) {}
+	CommonSubexpressionEliminator(KnownState const& _state): m_initialState(_state), m_state(_state) {}
 
 	/// Feeds AssemblyItems into the eliminator and @returns the iterator pointing at the first
 	/// item that must be fed into a new instance of the eliminator.
@@ -85,6 +85,7 @@ private:
 	/// Tries to optimize the item that breaks the basic block at the end.
 	void optimizeBreakingItem();
 
+	KnownState m_initialState;
 	KnownState m_state;
 	/// Keeps information about which storage or memory slots were written to at which sequence
 	/// number with what instruction.
@@ -115,6 +116,7 @@ public:
 	/// @param _targetStackContents final contents of the stack, by stack height relative to initial
 	/// @note should only be called once on each object.
 	AssemblyItems generateCode(
+		int _initialStackHeight,
 		std::map<int, Id> const& _initialStack,
 		std::map<int, Id> const& _targetStackContents
 	);
@@ -150,7 +152,7 @@ private:
 
 	AssemblyItems m_generatedItems;
 	/// Current height of the stack relative to the start.
-	int m_stackHeight = 0;
+	int m_stackHeight;
 	/// If (b, a) is in m_requests then b is needed to compute a.
 	std::multimap<Id, Id> m_neededBy;
 	/// Current content of the stack.

libevmasm/ExpressionClasses.cpp

@@ -79,15 +79,6 @@ ExpressionClasses::Id ExpressionClasses::find(
 	return exp.id;
 }
 
-ExpressionClasses::Id ExpressionClasses::newId()
-{
-	// Note that we cannot insert it in m_expressions because this requires item to be set.
-	Expression exp;
-	exp.id = m_representatives.size();
-	m_representatives.push_back(exp);
-	return exp.id;
-}
-
 bool ExpressionClasses::knownToBeDifferent(ExpressionClasses::Id _a, ExpressionClasses::Id _b)
 {
 	// Try to simplify "_a - _b" and return true iff the value is a non-zero constant.

libevmasm/ExpressionClasses.h

@@ -68,10 +68,6 @@ public:
 		bool _copyItem = true,
 		unsigned _sequenceNumber = 0
 	);
-	/// @returns a new unique class id which does not and will never have a representative containing
-	/// an AssemblyItem, i.e. its value cannot be generated, instead it has to be assumed to be
-	/// already present.
-	Id newId();
 	/// @returns the canonical representative of an expression class.
 	Expression const& representative(Id _id) const { return m_representatives.at(_id); }
 	/// @returns the number of classes.

libevmasm/KnownState.cpp

@@ -135,8 +135,10 @@ ExpressionClasses::Id KnownState::stackElement(int _stackHeight, SourceLocation
 {
 	if (m_stackElements.count(_stackHeight))
 		return m_stackElements.at(_stackHeight);
-	// Stack element not found (not assigned yet), create new equivalence class.
+	// Stack element not found (not assigned yet), create new unknown equivalence class.
-	return m_stackElements[_stackHeight] = m_expressionClasses->newId();
+	//@todo check that we do not infer incorrect equivalences when the stack is cleared partially
+	//in between.
+	return m_stackElements[_stackHeight] = initialStackElement(_stackHeight, _location);
 }
 
 ExpressionClasses::Id KnownState::initialStackElement(
@@ -144,10 +146,8 @@ ExpressionClasses::Id KnownState::initialStackElement(
 	SourceLocation const& _location
 )
 {
-	assertThrow(_stackHeight <= 0, OptimizerException, "Initial stack element of positive height requested.");
-	assertThrow(_stackHeight > -16, StackTooDeepException, "");
 	// This is a special assembly item that refers to elements pre-existing on the initial stack.
-	return m_expressionClasses->find(AssemblyItem(dupInstruction(1 - _stackHeight), _location));
+	return m_expressionClasses->find(AssemblyItem(UndefinedItem, u256(_stackHeight), _location));
 }
 
 void KnownState::setStackElement(int _stackHeight, Id _class)

test/libsolidity/SolidityOptimizer.cpp

@@ -83,15 +83,28 @@ public:
 							"\nOptimized:     " + toHex(optimizedOutput));
 	}
 
-	AssemblyItems getCSE(AssemblyItems const& _input)
+	AssemblyItems addDummyLocations(AssemblyItems const& _input)
 	{
 		// add dummy locations to each item so that we can check that they are not deleted
 		AssemblyItems input = _input;
 		for (AssemblyItem& item: input)
 			item.setLocation(SourceLocation(1, 3, make_shared<string>("")));
+		return input;
+	}
 
+	eth::KnownState createInitialState(AssemblyItems const& _input)
+	{
 		eth::KnownState state;
-		eth::CommonSubexpressionEliminator cse(state);
+		for (auto const& item: addDummyLocations(_input))
+			state.feedItem(item);
+		return state;
+	}
+
+	AssemblyItems getCSE(AssemblyItems const& _input, eth::KnownState const& _state = eth::KnownState())
+	{
+		AssemblyItems input = addDummyLocations(_input);
+
+		eth::CommonSubexpressionEliminator cse(_state);
 		BOOST_REQUIRE(cse.feedItems(input.begin(), input.end()) == input.end());
 		AssemblyItems output = cse.getOptimizedItems();
 
@@ -102,9 +115,13 @@ public:
 		return output;
 	}
 
-	void checkCSE(AssemblyItems const& _input, AssemblyItems const& _expectation)
+	void checkCSE(
+		AssemblyItems const& _input,
+		AssemblyItems const& _expectation,
+		KnownState const& _state = eth::KnownState()
+	)
 	{
-		AssemblyItems output = getCSE(_input);
+		AssemblyItems output = getCSE(_input, _state);
 		BOOST_CHECK_EQUAL_COLLECTIONS(_expectation.begin(), _expectation.end(), output.begin(), output.end());
 	}
 
@@ -756,6 +773,30 @@ BOOST_AUTO_TEST_CASE(cse_sha3_twice_same_content_noninterfering_store_in_between
 	BOOST_CHECK_EQUAL(1, count(output.begin(), output.end(), AssemblyItem(Instruction::SHA3)));
 }
 
+BOOST_AUTO_TEST_CASE(cse_with_initially_known_stack)
+{
+	eth::KnownState state = createInitialState(AssemblyItems{
+		u256(0x12),
+		u256(0x20),
+		Instruction::ADD
+	});
+	AssemblyItems input{
+		u256(0x12 + 0x20)
+	};
+	checkCSE(input, AssemblyItems{Instruction::DUP1}, state);
+}
+
+BOOST_AUTO_TEST_CASE(cse_equality_on_initially_known_stack)
+{
+	eth::KnownState state = createInitialState(AssemblyItems{Instruction::DUP1});
+	AssemblyItems input{
+		Instruction::EQ
+	};
+	AssemblyItems output = getCSE(input, state);
+	// check that it directly pushes 1 (true)
+	BOOST_CHECK(find(output.begin(), output.end(), AssemblyItem(u256(1))) != output.end());
+}
+
 BOOST_AUTO_TEST_CASE(control_flow_graph_remove_unused)
 {
 	// remove parts of the code that are unused