Change RematCandidateSelector to not depend on variable name sorting.

libyul/optimiser/StackCompressor.cpp

@@ -31,6 +31,8 @@
 
 #include <libyul/AST.h>
 
+#include <libsolutil/CommonData.h>
+
 using namespace std;
 using namespace solidity;
 using namespace solidity::yul;
@@ -47,16 +49,19 @@ class RematCandidateSelector: public DataFlowAnalyzer
 public:
 	explicit RematCandidateSelector(Dialect const& _dialect): DataFlowAnalyzer(_dialect) {}
 
-	/// @returns a set of tuples of rematerialisation costs, variable to rematerialise
+	/// @returns a map from rematerialisation costs to a vector of variables to rematerialise
-	/// and variables that occur in its expression.
+	/// and variables that occur in their expression.
-	/// Note that this set is sorted by cost.
+	/// While the map is sorted by cost, the contained vectors are sorted by the order of occurrence.
-	set<tuple<size_t, YulString, set<YulString>>> candidates()
+	map<size_t, vector<tuple<YulString, set<YulString>>>> candidates()
 	{
-		set<tuple<size_t, YulString, set<YulString>>> cand;
+		map<size_t, vector<tuple<YulString, set<YulString>>>> cand;
-		for (auto const& codeCost: m_expressionCodeCost)
+		for (auto const& candidate: m_candidates)
 		{
-			size_t numRef = m_numReferences[codeCost.first];
+			if (size_t const* cost = util::valueOrNullptr(m_expressionCodeCost, candidate))
-			cand.emplace(make_tuple(codeCost.second * numRef, codeCost.first, m_references[codeCost.first]));
+			{
+				size_t numRef = m_numReferences[candidate];
+				cand[*cost * numRef].emplace_back(candidate, m_references[candidate]);
+			}
 		}
 		return cand;
 	}
@@ -69,9 +74,13 @@ public:
 		{
 			YulString varName = _varDecl.variables.front().name;
 			if (m_value.count(varName))
+			{
+				yulAssert(!m_expressionCodeCost.count(varName), "");
+				m_candidates.emplace_back(varName);
 				m_expressionCodeCost[varName] = CodeCost::codeCost(m_dialect, *m_value[varName].value);
 			}
 		}
+	}
 
 	void operator()(Assignment& _assignment) override
 	{
@@ -105,12 +114,40 @@ public:
 		m_expressionCodeCost.erase(_variable);
 	}
 
+	/// All candidate variables in order of occurrence.
+	vector<YulString> m_candidates;
 	/// Candidate variables and the code cost of their value.
 	map<YulString, size_t> m_expressionCodeCost;
 	/// Number of references to each candidate variable.
 	map<YulString, size_t> m_numReferences;
 };
 
+/// Selects at most @a _numVariables among @a _candidates.
+set<YulString> chooseVarsToEliminate(
+	map<size_t, vector<tuple<YulString, set<YulString>>>> const& _candidates,
+	size_t _numVariables
+)
+{
+	set<YulString> varsToEliminate;
+	for (auto&& [cost, candidates]: _candidates)
+		for (auto&& [candidate, references]: candidates)
+		{
+			if (varsToEliminate.size() >= _numVariables)
+				return varsToEliminate;
+			// If a variable we would like to eliminate references another one
+			// we already selected for elimination, then stop selecting
+			// candidates. If we would add that variable, then the cost calculation
+			// for the previous variable would be off. Furthermore, we
+			// do not skip the variable because it would be better to properly re-compute
+			// the costs of all other variables instead.
+			for (YulString const& referencedVar: references)
+				if (varsToEliminate.count(referencedVar))
+					return varsToEliminate;
+			varsToEliminate.insert(candidate);
+		}
+	return varsToEliminate;
+}
+
 template <typename ASTNode>
 void eliminateVariables(
 	Dialect const& _dialect,
@@ -121,32 +158,7 @@ void eliminateVariables(
 {
 	RematCandidateSelector selector{_dialect};
 	selector(_node);
-
+	Rematerialiser::run(_dialect, _node, chooseVarsToEliminate(selector.candidates(), _numVariables));
-	// Select at most _numVariables
-	set<YulString> varsToEliminate;
-	for (auto const& costs: selector.candidates())
-	{
-		if (varsToEliminate.size() >= _numVariables)
-			break;
-		// If a variable we would like to eliminate references another one
-		// we already selected for elimination, then stop selecting
-		// candidates. If we would add that variable, then the cost calculation
-		// for the previous variable would be off. Furthermore, we
-		// do not skip the variable because it would be better to properly re-compute
-		// the costs of all other variables instead.
-		bool referencesVarToEliminate = false;
-		for (YulString const& referencedVar: get<2>(costs))
-			if (varsToEliminate.count(referencedVar))
-			{
-				referencesVarToEliminate = true;
-				break;
-			}
-		if (referencesVarToEliminate)
-			break;
-		varsToEliminate.insert(get<1>(costs));
-	}
-
-	Rematerialiser::run(_dialect, _node, std::move(varsToEliminate));
 	UnusedPruner::runUntilStabilised(_dialect, _node, _allowMSizeOptimization);
 }
 

test/libyul/yulOptimizerTests/fullSuite/aztec.yul

@@ -284,15 +284,17 @@
 //             let _5 := 0x40
 //             calldatacopy(0xe0, add(_3, 164), _5)
 //             calldatacopy(0x20, add(_3, 100), _5)
-//             mstore(0x120, sub(_2, c))
+//             let _6 := 0x120
-//             mstore(0x60, k)
+//             mstore(_6, sub(_2, c))
+//             let _7 := 0x60
+//             mstore(_7, k)
 //             mstore(0xc0, a)
-//             let result := call(gas(), 7, 0, 0xe0, 0x60, 0x1a0, _5)
+//             let result := call(gas(), 7, 0, 0xe0, _7, 0x1a0, _5)
-//             let result_1 := and(result, call(gas(), 7, 0, 0x20, 0x60, 0x120, _5))
+//             let result_1 := and(result, call(gas(), 7, 0, 0x20, _7, _6, _5))
-//             let _6 := 0x160
+//             let _8 := 0x160
-//             let result_2 := and(result_1, call(gas(), 7, 0, 0x80, 0x60, _6, _5))
+//             let result_2 := and(result_1, call(gas(), 7, 0, 0x80, _7, _8, _5))
-//             let result_3 := and(result_2, call(gas(), 6, 0, 0x120, 0x80, _6, _5))
+//             let result_3 := and(result_2, call(gas(), 6, 0, _6, 0x80, _8, _5))
-//             result := and(result_3, call(gas(), 6, 0, _6, 0x80, b, _5))
+//             result := and(result_3, call(gas(), 6, 0, _8, 0x80, b, _5))
 //             if eq(i, m)
 //             {
 //                 mstore(0x260, mload(0x20))
@@ -302,8 +304,8 @@
 //             }
 //             if gt(i, m)
 //             {
-//                 mstore(0x60, c)
+//                 mstore(_7, c)
-//                 let result_4 := and(result, call(gas(), 7, 0, 0x20, 0x60, 0x220, _5))
+//                 let result_4 := and(result, call(gas(), 7, 0, 0x20, _7, 0x220, _5))
 //                 let result_5 := and(result_4, call(gas(), 6, 0, 0x220, 0x80, 0x260, _5))
 //                 result := and(result_5, call(gas(), 6, 0, 0x1a0, 0x80, 0x1e0, _5))
 //             }