Merge pull request #12132 from ethereum/userDefinedSideEffectsCodeTransform

Use side-effects of user defined functions in evm code transform.

Author: ekpyron

Changelog.md

@@ -9,6 +9,7 @@ Compiler Features:
  * EVM: Basic support for the EVM version "Paris".
  * Natspec: Add event Natspec inheritance for devdoc.
  * Standard JSON: Add a boolean field `settings.metadata.appendCBOR` that skips CBOR metadata from getting appended at the end of the bytecode.
+ * Yul EVM Code Transform: Generate more optimal code for user-defined functions that always terminate a transaction. No return labels will be pushed for calls to functions that always terminate.
  * Yul Optimizer: Allow replacing the previously hard-coded cleanup sequence by specifying custom steps after a colon delimiter (``:``) in the sequence string.
  * Language Server: Add basic document hover support.
  * Optimizer: Added optimization rule ``and(shl(X, Y), shl(X, Z)) => shl(X, and(Y, Z))``.

libyul/backends/evm/ControlFlowGraph.h

@@ -148,6 +148,8 @@ struct CFG
 		/// True, if the call is recursive, i.e. entering the function involves a control flow path (potentially involving
 		/// more intermediate function calls) that leads back to this very call.
 		bool recursive = false;
+		/// True, if the call can return.
+		bool canContinue = true;
 	};
 	struct Assignment
 	{
@@ -210,10 +212,12 @@ struct CFG
 	{
 		std::shared_ptr<DebugData const> debugData;
 		Scope::Function const& function;
+		FunctionDefinition const& functionDefinition;
 		BasicBlock* entry = nullptr;
 		std::vector<VariableSlot> parameters;
 		std::vector<VariableSlot> returnVariables;
 		std::vector<BasicBlock*> exits;
+		bool canContinue = true;
 	};
 
 	/// The main entry point, i.e. the start of the outermost Yul block.

libyul/backends/evm/ControlFlowGraphBuilder.cpp

@@ -23,6 +23,7 @@
 #include <libyul/AST.h>
 #include <libyul/Exceptions.h>
 #include <libyul/Utilities.h>
+#include <libyul/ControlFlowSideEffectsCollector.h>
 
 #include <libsolutil/cxx20.h>
 #include <libsolutil/Visitor.h>
@@ -214,7 +215,8 @@ std::unique_ptr<CFG> ControlFlowGraphBuilder::build(
 	auto result = std::make_unique<CFG>();
 	result->entry = &result->makeBlock(debugDataOf(_block));
 
-	ControlFlowGraphBuilder builder(*result, _analysisInfo, _dialect);
+	ControlFlowSideEffectsCollector sideEffects(_dialect, _block);
+	ControlFlowGraphBuilder builder(*result, _analysisInfo, sideEffects.functionSideEffects(), _dialect);
 	builder.m_currentBlock = result->entry;
 	builder(_block);
 
@@ -232,10 +234,12 @@ std::unique_ptr<CFG> ControlFlowGraphBuilder::build(
 ControlFlowGraphBuilder::ControlFlowGraphBuilder(
 	CFG& _graph,
 	AsmAnalysisInfo const& _analysisInfo,
+	map<FunctionDefinition const*, ControlFlowSideEffects> const& _functionSideEffects,
 	Dialect const& _dialect
 ):
 	m_graph(_graph),
 	m_info(_analysisInfo),
+	m_functionSideEffects(_functionSideEffects),
 	m_dialect(_dialect)
 {
 }
@@ -285,10 +289,10 @@ void ControlFlowGraphBuilder::operator()(Assignment const& _assignment)
 		return VariableSlot{lookupVariable(_var.name), _var.debugData};
 	}) | ranges::to<vector<VariableSlot>>;
 
-	yulAssert(m_currentBlock, "");
+	Stack input = visitAssignmentRightHandSide(*_assignment.value, assignedVariables.size());
+	yulAssert(m_currentBlock);
 	m_currentBlock->operations.emplace_back(CFG::Operation{
-		// input
-		visitAssignmentRightHandSide(*_assignment.value, assignedVariables.size()),
+		std::move(input),
 		// output
 		assignedVariables | ranges::to<Stack>,
 		// operation
@@ -297,24 +301,13 @@ void ControlFlowGraphBuilder::operator()(Assignment const& _assignment)
 }
 void ControlFlowGraphBuilder::operator()(ExpressionStatement const& _exprStmt)
 {
-	yulAssert(m_currentBlock, "");
 	std::visit(util::GenericVisitor{
 		[&](FunctionCall const& _call) {
 			Stack const& output = visitFunctionCall(_call);
 			yulAssert(output.empty(), "");
 		},
 		[&](auto const&) { yulAssert(false, ""); }
 	}, _exprStmt.expression);
-
-	// TODO: Ideally this would be done on the expression label and for all functions that always revert,
-	//       not only for builtins.
-	if (auto const* funCall = get_if<FunctionCall>(&_exprStmt.expression))
-		if (BuiltinFunction const* builtin = m_dialect.builtin(funCall->functionName.name))
-			if (builtin->controlFlowSideEffects.terminatesOrReverts())
-			{
-				m_currentBlock->exit = CFG::BasicBlock::Terminated{};
-				m_currentBlock = &m_graph.makeBlock(debugDataOf(*m_currentBlock));
-			}
 }
 
 void ControlFlowGraphBuilder::operator()(Block const& _block)
@@ -331,7 +324,8 @@ void ControlFlowGraphBuilder::operator()(If const& _if)
 {
 	auto& ifBranch = m_graph.makeBlock(debugDataOf(_if.body));
 	auto& afterIf = m_graph.makeBlock(debugDataOf(*m_currentBlock));
-	makeConditionalJump(debugDataOf(_if), std::visit(*this, *_if.condition), ifBranch, afterIf);
+	StackSlot condition = std::visit(*this, *_if.condition);
+	makeConditionalJump(debugDataOf(_if), std::move(condition), ifBranch, afterIf);
 	m_currentBlock = &ifBranch;
 	(*this)(_if.body);
 	jump(debugDataOf(_if.body), afterIf);
@@ -349,8 +343,9 @@ void ControlFlowGraphBuilder::operator()(Switch const& _switch)
 	// Artificially generate:
 	// let <ghostVariable> := <switchExpression>
 	VariableSlot ghostVarSlot{ghostVar, debugDataOf(*_switch.expression)};
+	StackSlot expression = std::visit(*this, *_switch.expression);
 	m_currentBlock->operations.emplace_back(CFG::Operation{
-		Stack{std::visit(*this, *_switch.expression)},
+		Stack{std::move(expression)},
 		Stack{ghostVarSlot},
 		CFG::Assignment{_switch.debugData, {ghostVarSlot}}
 	});
@@ -430,7 +425,8 @@ void ControlFlowGraphBuilder::operator()(ForLoop const& _loop)
 	else
 	{
 		jump(debugDataOf(_loop.pre), loopCondition);
-		makeConditionalJump(debugDataOf(*_loop.condition), std::visit(*this, *_loop.condition), loopBody, afterLoop);
+		StackSlot condition = std::visit(*this, *_loop.condition);
+		makeConditionalJump(debugDataOf(*_loop.condition), std::move(condition), loopBody, afterLoop);
 		m_currentBlock = &loopBody;
 		(*this)(_loop.body);
 		jump(debugDataOf(_loop.body), post);
@@ -473,41 +469,43 @@ void ControlFlowGraphBuilder::operator()(FunctionDefinition const& _function)
 
 	CFG::FunctionInfo& functionInfo = m_graph.functionInfo.at(&function);
 
-	ControlFlowGraphBuilder builder{m_graph, m_info, m_dialect};
+	ControlFlowGraphBuilder builder{m_graph, m_info, m_functionSideEffects, m_dialect};
 	builder.m_currentFunction = &functionInfo;
 	builder.m_currentBlock = functionInfo.entry;
 	builder(_function.body);
 	functionInfo.exits.emplace_back(builder.m_currentBlock);
 	builder.m_currentBlock->exit = CFG::BasicBlock::FunctionReturn{debugDataOf(_function), &functionInfo};
 }
 
-void ControlFlowGraphBuilder::registerFunction(FunctionDefinition const& _function)
+void ControlFlowGraphBuilder::registerFunction(FunctionDefinition const& _functionDefinition)
 {
 	yulAssert(m_scope, "");
-	yulAssert(m_scope->identifiers.count(_function.name), "");
-	Scope::Function& function = std::get<Scope::Function>(m_scope->identifiers.at(_function.name));
+	yulAssert(m_scope->identifiers.count(_functionDefinition.name), "");
+	Scope::Function& function = std::get<Scope::Function>(m_scope->identifiers.at(_functionDefinition.name));
 
-	yulAssert(m_info.scopes.at(&_function.body), "");
-	Scope* virtualFunctionScope = m_info.scopes.at(m_info.virtualBlocks.at(&_function).get()).get();
+	yulAssert(m_info.scopes.at(&_functionDefinition.body), "");
+	Scope* virtualFunctionScope = m_info.scopes.at(m_info.virtualBlocks.at(&_functionDefinition).get()).get();
 	yulAssert(virtualFunctionScope, "");
 
 	bool inserted = m_graph.functionInfo.emplace(std::make_pair(&function, CFG::FunctionInfo{
-		_function.debugData,
+		_functionDefinition.debugData,
 		function,
-		&m_graph.makeBlock(debugDataOf(_function.body)),
-		_function.parameters | ranges::views::transform([&](auto const& _param) {
+		_functionDefinition,
+		&m_graph.makeBlock(debugDataOf(_functionDefinition.body)),
+		_functionDefinition.parameters | ranges::views::transform([&](auto const& _param) {
 			return VariableSlot{
 				std::get<Scope::Variable>(virtualFunctionScope->identifiers.at(_param.name)),
 				_param.debugData
 			};
 		}) | ranges::to<vector>,
-		_function.returnVariables | ranges::views::transform([&](auto const& _retVar) {
+		_functionDefinition.returnVariables | ranges::views::transform([&](auto const& _retVar) {
 			return VariableSlot{
 				std::get<Scope::Variable>(virtualFunctionScope->identifiers.at(_retVar.name)),
 				_retVar.debugData
 			};
 		}) | ranges::to<vector>,
-		{}
+		{},
+		m_functionSideEffects.at(&_functionDefinition).canContinue
 	})).second;
 	yulAssert(inserted);
 }
@@ -517,14 +515,16 @@ Stack const& ControlFlowGraphBuilder::visitFunctionCall(FunctionCall const& _cal
 	yulAssert(m_scope, "");
 	yulAssert(m_currentBlock, "");
 
+	Stack const* output = nullptr;
+	bool canContinue = true;
 	if (BuiltinFunction const* builtin = m_dialect.builtin(_call.functionName.name))
 	{
 		Stack inputs;
 		for (auto&& [idx, arg]: _call.arguments | ranges::views::enumerate | ranges::views::reverse)
 			if (!builtin->literalArgument(idx).has_value())
 				inputs.emplace_back(std::visit(*this, arg));
 		CFG::BuiltinCall builtinCall{_call.debugData, *builtin, _call, inputs.size()};
-		return m_currentBlock->operations.emplace_back(CFG::Operation{
+		output = &m_currentBlock->operations.emplace_back(CFG::Operation{
 			// input
 			std::move(inputs),
 			// output
@@ -534,24 +534,34 @@ Stack const& ControlFlowGraphBuilder::visitFunctionCall(FunctionCall const& _cal
 			// operation
 			std::move(builtinCall)
 		}).output;
+		canContinue = builtin->controlFlowSideEffects.canContinue;
 	}
 	else
 	{
 		Scope::Function const& function = lookupFunction(_call.functionName.name);
-		Stack inputs{FunctionCallReturnLabelSlot{_call}};
+		canContinue = m_graph.functionInfo.at(&function).canContinue;
+		Stack inputs;
+		if (canContinue)
+			inputs.emplace_back(FunctionCallReturnLabelSlot{_call});
 		for (auto const& arg: _call.arguments | ranges::views::reverse)
 			inputs.emplace_back(std::visit(*this, arg));
-		return m_currentBlock->operations.emplace_back(CFG::Operation{
+		output = &m_currentBlock->operations.emplace_back(CFG::Operation{
 			// input
 			std::move(inputs),
 			// output
 			ranges::views::iota(0u, function.returns.size()) | ranges::views::transform([&](size_t _i) {
 				return TemporarySlot{_call, _i};
 			}) | ranges::to<Stack>,
 			// operation
-			CFG::FunctionCall{_call.debugData, function, _call}
+			CFG::FunctionCall{_call.debugData, function, _call, /* recursive */ false, canContinue}
 		}).output;
 	}
+	if (!canContinue)
+	{
+		m_currentBlock->exit = CFG::BasicBlock::Terminated{};
+		m_currentBlock = &m_graph.makeBlock(debugDataOf(*m_currentBlock));
+	}
+	return *output;
 }
 
 Stack ControlFlowGraphBuilder::visitAssignmentRightHandSide(Expression const& _expression, size_t _expectedSlotCount)

libyul/backends/evm/ControlFlowGraphBuilder.h

@@ -21,6 +21,7 @@
 #pragma once
 
 #include <libyul/backends/evm/ControlFlowGraph.h>
+#include <libyul/ControlFlowSideEffects.h>
 
 namespace solidity::yul
 {
@@ -55,6 +56,7 @@ class ControlFlowGraphBuilder
 	ControlFlowGraphBuilder(
 		CFG& _graph,
 		AsmAnalysisInfo const& _analysisInfo,
+		std::map<FunctionDefinition const*, ControlFlowSideEffects> const& _functionSideEffects,
 		Dialect const& _dialect
 	);
 	void registerFunction(FunctionDefinition const& _function);
@@ -77,6 +79,7 @@ class ControlFlowGraphBuilder
 	);
 	CFG& m_graph;
 	AsmAnalysisInfo const& m_info;
+	std::map<FunctionDefinition const*, ControlFlowSideEffects> const& m_functionSideEffects;
 	Dialect const& m_dialect;
 	CFG::BasicBlock* m_currentBlock = nullptr;
 	Scope* m_scope = nullptr;

libyul/backends/evm/OptimizedEVMCodeTransform.cpp

@@ -71,35 +71,39 @@ void OptimizedEVMCodeTransform::operator()(CFG::FunctionCall const& _call)
 	// Validate stack.
 	{
 		yulAssert(m_assembly.stackHeight() == static_cast<int>(m_stack.size()), "");
-		yulAssert(m_stack.size() >= _call.function.get().arguments.size() + 1, "");
+		yulAssert(m_stack.size() >= _call.function.get().arguments.size() + (_call.canContinue ? 1 : 0), "");
 		// Assert that we got the correct arguments on stack for the call.
 		for (auto&& [arg, slot]: ranges::zip_view(
 			_call.functionCall.get().arguments | ranges::views::reverse,
 			m_stack | ranges::views::take_last(_call.functionCall.get().arguments.size())
 		))
 			validateSlot(slot, arg);
 		// Assert that we got the correct return label on stack.
-		auto const* returnLabelSlot = get_if<FunctionCallReturnLabelSlot>(
-			&m_stack.at(m_stack.size() - _call.functionCall.get().arguments.size() - 1)
-		);
-		yulAssert(returnLabelSlot && &returnLabelSlot->call.get() == &_call.functionCall.get(), "");
+		if (_call.canContinue)
+		{
+			auto const* returnLabelSlot = get_if<FunctionCallReturnLabelSlot>(
+				&m_stack.at(m_stack.size() - _call.functionCall.get().arguments.size() - 1)
+			);
+			yulAssert(returnLabelSlot && &returnLabelSlot->call.get() == &_call.functionCall.get(), "");
+		}
 	}
 
 	// Emit code.
 	{
 		m_assembly.setSourceLocation(originLocationOf(_call));
 		m_assembly.appendJumpTo(
 			getFunctionLabel(_call.function),
-			static_cast<int>(_call.function.get().returns.size() - _call.function.get().arguments.size()) - 1,
+			static_cast<int>(_call.function.get().returns.size() - _call.function.get().arguments.size()) - (_call.canContinue ? 1 : 0),
 			AbstractAssembly::JumpType::IntoFunction
 		);
-		m_assembly.appendLabel(m_returnLabels.at(&_call.functionCall.get()));
+		if (_call.canContinue)
+			m_assembly.appendLabel(m_returnLabels.at(&_call.functionCall.get()));
 	}
 
 	// Update stack.
 	{
 		// Remove arguments and return label from m_stack.
-		for (size_t i = 0; i < _call.function.get().arguments.size() + 1; ++i)
+		for (size_t i = 0; i < _call.function.get().arguments.size() + (_call.canContinue ? 1 : 0); ++i)
 			m_stack.pop_back();
 		// Push return values to m_stack.
 		for (size_t index: ranges::views::iota(0u, _call.function.get().returns.size()))
@@ -479,8 +483,9 @@ void OptimizedEVMCodeTransform::operator()(CFG::BasicBlock const& _block)
 		},
 		[&](CFG::BasicBlock::FunctionReturn const& _functionReturn)
 		{
-			yulAssert(m_currentFunctionInfo, "");
-			yulAssert(m_currentFunctionInfo == _functionReturn.info, "");
+			yulAssert(m_currentFunctionInfo);
+			yulAssert(m_currentFunctionInfo == _functionReturn.info);
+			yulAssert(m_currentFunctionInfo->canContinue);
 
 			// Construct the function return layout, which is fully determined by the function signature.
 			Stack exitStack = m_currentFunctionInfo->returnVariables | ranges::views::transform([](auto const& _varSlot){
@@ -494,11 +499,13 @@ void OptimizedEVMCodeTransform::operator()(CFG::BasicBlock const& _block)
 		},
 		[&](CFG::BasicBlock::Terminated const&)
 		{
-			// Assert that the last builtin call was in fact terminating.
-			yulAssert(!_block.operations.empty(), "");
-			CFG::BuiltinCall const* builtinCall = get_if<CFG::BuiltinCall>(&_block.operations.back().operation);
-			yulAssert(builtinCall, "");
-			yulAssert(builtinCall->builtin.get().controlFlowSideEffects.terminatesOrReverts(), "");
+			yulAssert(!_block.operations.empty());
+			if (CFG::BuiltinCall const* builtinCall = get_if<CFG::BuiltinCall>(&_block.operations.back().operation))
+				yulAssert(builtinCall->builtin.get().controlFlowSideEffects.terminatesOrReverts(), "");
+			else if (CFG::FunctionCall const* functionCall = get_if<CFG::FunctionCall>(&_block.operations.back().operation))
+				yulAssert(!functionCall->canContinue);
+			else
+				yulAssert(false);
 		}
 	}, _block.exit);
 	// TODO: We could assert that the last emitted assembly item terminated or was an (unconditional) jump.
@@ -515,7 +522,8 @@ void OptimizedEVMCodeTransform::operator()(CFG::FunctionInfo const& _functionInf
 	yulAssert(m_stack.empty() && m_assembly.stackHeight() == 0, "");
 
 	// Create function entry layout in m_stack.
-	m_stack.emplace_back(FunctionReturnLabelSlot{_functionInfo.function});
+	if (_functionInfo.canContinue)
+		m_stack.emplace_back(FunctionReturnLabelSlot{_functionInfo.function});
 	for (auto const& param: _functionInfo.parameters | ranges::views::reverse)
 		m_stack.emplace_back(param);
 	m_assembly.setStackHeight(static_cast<int>(m_stack.size()));