Merge pull request #6463 from ethereum/fixSSAMulti

Fix SSA for multi-assignments.

Changelog.md

@@ -1,5 +1,9 @@
 ### 0.5.8 (unreleased)
 
+Important Bugfixes:
+ * Yul Optimizer: Fix SSA transform for multi-assignments.
+
+
 Language Features:
  * Code Generation: Implement copying recursive structs from storage to memory.
 

libyul/optimiser/SSATransform.cpp

@@ -62,17 +62,13 @@ void SSATransform::operator()(Block& _block)
 {
 	set<YulString> variablesToClearAtEnd;
 
-	// Creates a new variable (and returns its declaration) with value _value
-	// and replaces _value by a reference to that new variable.
-
-	auto replaceByNew = [&](SourceLocation _loc, YulString _varName, YulString _type, unique_ptr<Expression>& _value) -> VariableDeclaration
+	// Creates a new variable and stores it in the current variable value map.
+	auto newVariable = [&](YulString _varName) -> YulString
 	{
 		YulString newName = m_nameDispenser.newName(_varName);
 		m_currentVariableValues[_varName] = newName;
 		variablesToClearAtEnd.emplace(_varName);
-		unique_ptr<Expression> v = make_unique<Expression>(Identifier{_loc, newName});
-		_value.swap(v);
-		return VariableDeclaration{_loc, {TypedName{_loc, std::move(newName), std::move(_type)}}, std::move(v)};
+		return newName;
 	};
 
 	iterateReplacing(
@@ -84,36 +80,60 @@ void SSATransform::operator()(Block& _block)
 				VariableDeclaration& varDecl = boost::get<VariableDeclaration>(_s);
 				if (varDecl.value)
 					visit(*varDecl.value);
-				if (varDecl.variables.size() != 1 || !m_variablesToReplace.count(varDecl.variables.front().name))
+
+				bool needToReplaceSome = false;
+				for (auto const& var: varDecl.variables)
+					if (m_variablesToReplace.count(var.name))
+						needToReplaceSome = true;
+				if (!needToReplaceSome)
 					return {};
-				vector<Statement> v;
+
 				// Replace "let a := v" by "let a_1 := v  let a := a_1"
-				v.emplace_back(replaceByNew(
-					varDecl.location,
-					varDecl.variables.front().name,
-					varDecl.variables.front().type,
-					varDecl.value
-				));
-				v.emplace_back(move(varDecl));
-				return std::move(v);
+				// Replace "let a, b := v" by "let a_1, b_1 := v  let a := a_1 let b := b_2"
+				auto loc = varDecl.location;
+				vector<Statement> statements;
+				statements.emplace_back(VariableDeclaration{loc, {}, std::move(varDecl.value)});
+				TypedNameList newVariables;
+				for (auto const& var: varDecl.variables)
+				{
+					YulString newName = newVariable(var.name);
+					YulString oldName = var.name;
+					newVariables.emplace_back(TypedName{loc, newName, {}});
+					statements.emplace_back(VariableDeclaration{
+						loc,
+						{TypedName{loc, oldName, {}}},
+						make_unique<Expression>(Identifier{loc, newName})
+					});
+				}
+				boost::get<VariableDeclaration>(statements.front()).variables = std::move(newVariables);
+				return std::move(statements);
 			}
 			else if (_s.type() == typeid(Assignment))
 			{
 				Assignment& assignment = boost::get<Assignment>(_s);
 				visit(*assignment.value);
-				if (assignment.variableNames.size() != 1)
-					return {};
-				assertThrow(m_variablesToReplace.count(assignment.variableNames.front().name), OptimizerException, "");
-				vector<Statement> v;
+				for (auto const& var: assignment.variableNames)
+					assertThrow(m_variablesToReplace.count(var.name), OptimizerException, "");
+
 				// Replace "a := v" by "let a_1 := v  a := v"
-				v.emplace_back(replaceByNew(
-					assignment.location,
-					assignment.variableNames.front().name,
-					{}, // TODO determine type
-					assignment.value
-				));
-				v.emplace_back(move(assignment));
-				return std::move(v);
+				// Replace "a, b := v" by "let a_1, b_1 := v  a := a_1 b := b_2"
+				auto loc = assignment.location;
+				vector<Statement> statements;
+				statements.emplace_back(VariableDeclaration{loc, {}, std::move(assignment.value)});
+				TypedNameList newVariables;
+				for (auto const& var: assignment.variableNames)
+				{
+					YulString newName = newVariable(var.name);
+					YulString oldName = var.name;
+					newVariables.emplace_back(TypedName{loc, newName, {}});
+					statements.emplace_back(Assignment{
+						loc,
+						{Identifier{loc, oldName}},
+						make_unique<Expression>(Identifier{loc, newName})
+					});
+				}
+				boost::get<VariableDeclaration>(statements.front()).variables = std::move(newVariables);
+				return std::move(statements);
 			}
 			else
 				visit(_s);

libyul/optimiser/SSATransform.h

@@ -87,6 +87,8 @@ private:
 	{ }
 
 	NameDispenser& m_nameDispenser;
+	/// This is a set of all variables that are assigned to anywhere in the code.
+	/// Variables that are only declared but never re-assigned are not touched.
 	std::set<YulString> const& m_variablesToReplace;
 	std::map<YulString, YulString> m_currentVariableValues;
 };

test/libyul/yulOptimizerTests/fullSuite/ssaReverse.yul

@@ -36,13 +36,13 @@
 // ----
 // {
 //     let a, b := abi_decode_t_bytes_calldata_ptr(mload(0), mload(1))
-//     a, b := abi_decode_t_bytes_calldata_ptr(a, b)
-//     a, b := abi_decode_t_bytes_calldata_ptr(a, b)
-//     a, b := abi_decode_t_bytes_calldata_ptr(a, b)
-//     a, b := abi_decode_t_bytes_calldata_ptr(a, b)
-//     a, b := abi_decode_t_bytes_calldata_ptr(a, b)
-//     a, b := abi_decode_t_bytes_calldata_ptr(a, b)
-//     mstore(a, b)
+//     let a_1, b_1 := abi_decode_t_bytes_calldata_ptr(a, b)
+//     let a_2, b_2 := abi_decode_t_bytes_calldata_ptr(a_1, b_1)
+//     let a_3, b_3 := abi_decode_t_bytes_calldata_ptr(a_2, b_2)
+//     let a_4, b_4 := abi_decode_t_bytes_calldata_ptr(a_3, b_3)
+//     let a_5, b_5 := abi_decode_t_bytes_calldata_ptr(a_4, b_4)
+//     let a_6, b_6 := abi_decode_t_bytes_calldata_ptr(a_5, b_5)
+//     mstore(a_6, b_6)
 //     function abi_decode_t_bytes_calldata_ptr(offset, end) -> arrayPos, length
 //     {
 //         if iszero(slt(add(offset, 0x1f), end))

test/libyul/yulOptimizerTests/ssaTransform/multi_assign.yul

@@ -0,0 +1,29 @@
+{
+  let a := mload(0)
+  let b := mload(1)
+  a, b := f()
+  sstore(a, b)
+  a := mload(5)
+  b := mload(a)
+  function f() -> x, y {}
+}
+// ====
+// step: ssaTransform
+// ----
+// {
+//     let a_1 := mload(0)
+//     let a := a_1
+//     let b_2 := mload(1)
+//     let b := b_2
+//     let a_3, b_4 := f()
+//     a := a_3
+//     b := b_4
+//     sstore(a_3, b_4)
+//     let a_5 := mload(5)
+//     a := a_5
+//     let b_6 := mload(a_5)
+//     b := b_6
+//     function f() -> x, y
+//     {
+//     }
+// }

test/libyul/yulOptimizerTests/ssaTransform/multi_decl.yul

@@ -0,0 +1,25 @@
+{
+  let x, y := f(1, 2)
+  x := mload(y)
+  y := mload(x)
+  let a, b := f(x, y)
+  sstore(a, b)
+  function f(t, v) -> x, y {}
+}
+// ====
+// step: ssaTransform
+// ----
+// {
+//     let x_2, y_3 := f(1, 2)
+//     let x := x_2
+//     let y := y_3
+//     let x_4 := mload(y_3)
+//     x := x_4
+//     let y_5 := mload(x_4)
+//     y := y_5
+//     let a, b := f(x_4, y_5)
+//     sstore(a, b)
+//     function f(t, v) -> x_1, y_2
+//     {
+//     }
+// }