Merge pull request #12796 from ethereum/refactorStackCompressor

Refactor stack compressor

libyul/optimiser/NameCollector.cpp

@@ -30,18 +30,22 @@ using namespace solidity::util;
 
 void NameCollector::operator()(VariableDeclaration const& _varDecl)
 {
-	for (auto const& var: _varDecl.variables)
+	if (m_collectWhat != OnlyFunctions)
-		m_names.emplace(var.name);
+		for (auto const& var: _varDecl.variables)
+			m_names.emplace(var.name);
 }
 
-void NameCollector::operator ()(FunctionDefinition const& _funDef)
+void NameCollector::operator()(FunctionDefinition const& _funDef)
 {
-	if (m_collectWhat == VariablesAndFunctions)
+	if (m_collectWhat != OnlyVariables)
 		m_names.emplace(_funDef.name);
-	for (auto const& arg: _funDef.parameters)
+	if (m_collectWhat != OnlyFunctions)
-		m_names.emplace(arg.name);
+	{
-	for (auto const& ret: _funDef.returnVariables)
+		for (auto const& arg: _funDef.parameters)
-		m_names.emplace(ret.name);
+			m_names.emplace(arg.name);
+		for (auto const& ret: _funDef.returnVariables)
+			m_names.emplace(ret.name);
+	}
 	ASTWalker::operator ()(_funDef);
 }
 

libyul/optimiser/NameCollector.h

@@ -35,7 +35,7 @@ namespace solidity::yul
 class NameCollector: public ASTWalker
 {
 public:
-	enum CollectWhat { VariablesAndFunctions, OnlyVariables };
+	enum CollectWhat { VariablesAndFunctions, OnlyVariables, OnlyFunctions };
 
 	explicit NameCollector(
 		Block const& _block,

libyul/optimiser/Rematerialiser.cpp

@@ -37,16 +37,6 @@ void Rematerialiser::run(Dialect const& _dialect, Block& _ast, set<YulString> _v
 	Rematerialiser{_dialect, _ast, std::move(_varsToAlwaysRematerialize), _onlySelectedVariables}(_ast);
 }
 
-void Rematerialiser::run(
-	Dialect const& _dialect,
-	FunctionDefinition& _function,
-	set<YulString> _varsToAlwaysRematerialize,
-	bool _onlySelectedVariables
-)
-{
-	Rematerialiser{_dialect, _function, std::move(_varsToAlwaysRematerialize), _onlySelectedVariables}(_function);
-}
-
 Rematerialiser::Rematerialiser(
 	Dialect const& _dialect,
 	Block& _ast,
@@ -60,19 +50,6 @@ Rematerialiser::Rematerialiser(
 {
 }
 
-Rematerialiser::Rematerialiser(
-	Dialect const& _dialect,
-	FunctionDefinition& _function,
-	set<YulString> _varsToAlwaysRematerialize,
-	bool _onlySelectedVariables
-):
-	DataFlowAnalyzer(_dialect),
-	m_referenceCounts(ReferencesCounter::countReferences(_function)),
-	m_varsToAlwaysRematerialize(std::move(_varsToAlwaysRematerialize)),
-	m_onlySelectedVariables(_onlySelectedVariables)
-{
-}
-
 void Rematerialiser::visit(Expression& _e)
 {
 	if (holds_alternative<Identifier>(_e))

libyul/optimiser/Rematerialiser.h

@@ -53,12 +53,6 @@ public:
 		std::set<YulString> _varsToAlwaysRematerialize = {},
 		bool _onlySelectedVariables = false
 	);
-	static void run(
-		Dialect const& _dialect,
-		FunctionDefinition& _function,
-		std::set<YulString> _varsToAlwaysRematerialize = {},
-		bool _onlySelectedVariables = false
-	);
 
 protected:
 	Rematerialiser(
@@ -67,12 +61,6 @@ protected:
 		std::set<YulString> _varsToAlwaysRematerialize = {},
 		bool _onlySelectedVariables = false
 	);
-	Rematerialiser(
-		Dialect const& _dialect,
-		FunctionDefinition& _function,
-		std::set<YulString> _varsToAlwaysRematerialize = {},
-		bool _onlySelectedVariables = false
-	);
 
 	using DataFlowAnalyzer::operator();
 

libyul/optimiser/StackCompressor.cpp

@@ -50,31 +50,41 @@ namespace
 /**
  * Class that discovers all variables that can be fully eliminated by rematerialization,
  * and the corresponding approximate costs.
+ *
+ * Prerequisite: Disambiguator, Function Grouper
  */
 class RematCandidateSelector: public DataFlowAnalyzer
 {
 public:
 	explicit RematCandidateSelector(Dialect const& _dialect): DataFlowAnalyzer(_dialect) {}
 
-	/// @returns a map from rematerialisation costs to a vector of variables to rematerialise
+	/// @returns a map from function name to rematerialisation costs to a vector of variables to rematerialise
 	/// and variables that occur in their expression.
 	/// While the map is sorted by cost, the contained vectors are sorted by the order of occurrence.
-	map<size_t, vector<tuple<YulString, set<YulString>>>> candidates()
+	map<YulString, map<size_t, vector<tuple<YulString, set<YulString>>>>> candidates()
 	{
-		map<size_t, vector<tuple<YulString, set<YulString>>>> cand;
+		map<YulString, map<size_t, vector<tuple<YulString, set<YulString>>>>> cand;
-		for (auto const& candidate: m_candidates)
+		for (auto const& [functionName, candidate]: m_candidates)
 		{
 			if (size_t const* cost = util::valueOrNullptr(m_expressionCodeCost, candidate))
 			{
 				size_t numRef = m_numReferences[candidate];
 				set<YulString> const* ref = references(candidate);
-				cand[*cost * numRef].emplace_back(candidate, ref ? move(*ref) : set<YulString>{});
+				cand[functionName][*cost * numRef].emplace_back(candidate, ref ? move(*ref) : set<YulString>{});
 			}
 		}
 		return cand;
 	}
 
 	using DataFlowAnalyzer::operator();
+	void operator()(FunctionDefinition& _function) override
+	{
+		yulAssert(m_currentFunctionName.empty());
+		m_currentFunctionName = _function.name;
+		DataFlowAnalyzer::operator()(_function);
+		m_currentFunctionName = {};
+	}
+
 	void operator()(VariableDeclaration& _varDecl) override
 	{
 		DataFlowAnalyzer::operator()(_varDecl);
@@ -84,7 +94,7 @@ public:
 			if (AssignedValue const* value = variableValue(varName))
 			{
 				yulAssert(!m_expressionCodeCost.count(varName), "");
-				m_candidates.emplace_back(varName);
+				m_candidates.emplace_back(m_currentFunctionName, varName);
 				m_expressionCodeCost[varName] = CodeCost::codeCost(m_dialect, *value->value);
 			}
 		}
@@ -122,8 +132,10 @@ public:
 		m_expressionCodeCost.erase(_variable);
 	}
 
-	/// All candidate variables in order of occurrence.
+	YulString m_currentFunctionName = {};
-	vector<YulString> m_candidates;
+
+	/// All candidate variables by function name, in order of occurrence.
+	vector<pair<YulString, 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.
@@ -156,62 +168,80 @@ set<YulString> chooseVarsToEliminate(
 	return varsToEliminate;
 }
 
-template <typename ASTNode>
 void eliminateVariables(
 	Dialect const& _dialect,
-	ASTNode& _node,
+	Block& _ast,
-	size_t _numVariables,
+	map<YulString, int> const& _numVariables,
 	bool _allowMSizeOptimization
 )
 {
 	RematCandidateSelector selector{_dialect};
-	selector(_node);
+	selector(_ast);
-	Rematerialiser::run(_dialect, _node, chooseVarsToEliminate(selector.candidates(), _numVariables));
+	map<YulString, map<size_t, vector<tuple<YulString, set<YulString>>>>> candidates = selector.candidates();
-	UnusedPruner::runUntilStabilised(_dialect, _node, _allowMSizeOptimization);
+
+	set<YulString> varsToEliminate;
+	for (auto const& [functionName, numVariables]: _numVariables)
+	{
+		yulAssert(numVariables > 0);
+		varsToEliminate += chooseVarsToEliminate(candidates[functionName], static_cast<size_t>(numVariables));
+	}
+
+	Rematerialiser::run(_dialect, _ast, move(varsToEliminate));
+	// Do not remove functions.
+	set<YulString> allFunctions = NameCollector{_ast, NameCollector::OnlyFunctions}.names();
+	UnusedPruner::runUntilStabilised(_dialect, _ast, _allowMSizeOptimization, nullptr, allFunctions);
 }
 
-void eliminateVariables(
+void eliminateVariablesOptimizedCodegen(
 	Dialect const& _dialect,
-	Block& _block,
+	Block& _ast,
-	vector<StackLayoutGenerator::StackTooDeep> const& _unreachables,
+	map<YulString, vector<StackLayoutGenerator::StackTooDeep>> const& _unreachables,
 	bool _allowMSizeOptimization
 )
 {
+	if (std::all_of(_unreachables.begin(), _unreachables.end(), [](auto const& _item) { return _item.second.empty(); }))
+		return;
+
 	RematCandidateSelector selector{_dialect};
-	selector(_block);
+	selector(_ast);
-	std::map<YulString, size_t> candidates;
+
-	for (auto [cost, candidatesWithCost]: selector.candidates())
+	map<YulString, size_t> candidates;
-		for (auto candidate: candidatesWithCost)
+	for (auto const& [functionName, candidatesInFunction]: selector.candidates())
-			candidates[get<0>(candidate)] = cost;
+		for (auto [cost, candidatesWithCost]: candidatesInFunction)
+			for (auto candidate: candidatesWithCost)
+				candidates[get<0>(candidate)] = cost;
 
 	set<YulString> varsToEliminate;
 
 	// TODO: this currently ignores the fact that variables may reference other variables we want to eliminate.
-	for (auto const& unreachable: _unreachables)
+	for (auto const& [functionName, unreachables]: _unreachables)
-	{
+		for (auto const& unreachable: unreachables)
-		map<size_t, vector<YulString>> suitableCandidates;
-		size_t neededSlots = unreachable.deficit;
-		for (auto varName: unreachable.variableChoices)
 		{
-			if (varsToEliminate.count(varName))
+			map<size_t, vector<YulString>> suitableCandidates;
-				--neededSlots;
+			size_t neededSlots = unreachable.deficit;
-			else if (size_t* cost = util::valueOrNullptr(candidates, varName))
+			for (auto varName: unreachable.variableChoices)
-				if (!util::contains(suitableCandidates[*cost], varName))
+			{
-					suitableCandidates[*cost].emplace_back(varName);
+				if (varsToEliminate.count(varName))
-		}
+					--neededSlots;
-		for (auto candidatesByCost: suitableCandidates)
+				else if (size_t* cost = util::valueOrNullptr(candidates, varName))
-		{
+					if (!util::contains(suitableCandidates[*cost], varName))
-			for (auto candidate: candidatesByCost.second)
+						suitableCandidates[*cost].emplace_back(varName);
-				if (neededSlots--)
+			}
-					varsToEliminate.emplace(candidate);
+			for (auto candidatesByCost: suitableCandidates)
-				else
+			{
+				for (auto candidate: candidatesByCost.second)
+					if (neededSlots--)
+						varsToEliminate.emplace(candidate);
+					else
+						break;
+				if (!neededSlots)
 					break;
-			if (!neededSlots)
+			}
-				break;
 		}
-	}
+	Rematerialiser::run(_dialect, _ast, std::move(varsToEliminate), true);
-	Rematerialiser::run(_dialect, _block, std::move(varsToEliminate), true);
+	// Do not remove functions.
-	UnusedPruner::runUntilStabilised(_dialect, _block, _allowMSizeOptimization);
+	set<YulString> allFunctions = NameCollector{_ast, NameCollector::OnlyFunctions}.names();
+	UnusedPruner::runUntilStabilised(_dialect, _ast, _allowMSizeOptimization, nullptr, allFunctions);
 }
 
 }
@@ -239,21 +269,12 @@ bool StackCompressor::run(
 	{
 		yul::AsmAnalysisInfo analysisInfo = yul::AsmAnalyzer::analyzeStrictAssertCorrect(_dialect, _object);
 		unique_ptr<CFG> cfg = ControlFlowGraphBuilder::build(analysisInfo, _dialect, *_object.code);
-		Block& mainBlock = std::get<Block>(_object.code->statements.at(0));
+		eliminateVariablesOptimizedCodegen(
-		if (
+			_dialect,
-			auto stackTooDeepErrors = StackLayoutGenerator::reportStackTooDeep(*cfg, YulString{});
+			*_object.code,
-			!stackTooDeepErrors.empty()
+			StackLayoutGenerator::reportStackTooDeep(*cfg),
-		)
+			allowMSizeOptimzation
-			eliminateVariables(_dialect, mainBlock, stackTooDeepErrors, allowMSizeOptimzation);
+		);
-		for (size_t i = 1; i < _object.code->statements.size(); ++i)
-		{
-			auto& fun = std::get<FunctionDefinition>(_object.code->statements[i]);
-			if (
-				auto stackTooDeepErrors = StackLayoutGenerator::reportStackTooDeep(*cfg, fun.name);
-				!stackTooDeepErrors.empty()
-			)
-				eliminateVariables(_dialect, fun.body, stackTooDeepErrors, allowMSizeOptimzation);
-		}
 	}
 	else
 		for (size_t iterations = 0; iterations < _maxIterations; iterations++)
@@ -261,32 +282,12 @@ bool StackCompressor::run(
 			map<YulString, int> stackSurplus = CompilabilityChecker(_dialect, _object, _optimizeStackAllocation).stackDeficit;
 			if (stackSurplus.empty())
 				return true;
-
+			eliminateVariables(
-			if (stackSurplus.count(YulString{}))
+				_dialect,
-			{
+				*_object.code,
-				yulAssert(stackSurplus.at({}) > 0, "Invalid surplus value.");
+				stackSurplus,
-				eliminateVariables(
+				allowMSizeOptimzation
-					_dialect,
+			);
-					std::get<Block>(_object.code->statements.at(0)),
-					static_cast<size_t>(stackSurplus.at({})),
-					allowMSizeOptimzation
-				);
-			}
-
-			for (size_t i = 1; i < _object.code->statements.size(); ++i)
-			{
-				auto& fun = std::get<FunctionDefinition>(_object.code->statements[i]);
-				if (!stackSurplus.count(fun.name))
-					continue;
-
-				yulAssert(stackSurplus.at(fun.name) > 0, "Invalid surplus value.");
-				eliminateVariables(
-					_dialect,
-					fun,
-					static_cast<size_t>(stackSurplus.at(fun.name)),
-					allowMSizeOptimzation
-				);
-			}
 		}
 	return false;
 }

test/libyul/yulOptimizerTests/stackCompressor/inlineInFunction.yul

@@ -11,7 +11,6 @@
 // step: stackCompressor
 //
 // {
-//     { let x := 8 }
 //     function f()
 //     {
 //         mstore(calldataload(calldataload(9)), add(add(add(add(add(add(add(add(add(add(add(add(add(add(add(add(add(add(calldataload(calldataload(9)), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1), 1))