ssa cfg: remove FunctionCall yul AST pointers from (Builtin)Calls and drop ghost calls

Author: clonker

libyul/backends/evm/ssa/CodeTransform.cpp

@@ -208,7 +208,7 @@ void CodeTransform::operator()(SSACFG::OperationId _opId, StackData const& _oper
 
 	if (hasReturnLabel)
 	{
-		auto const [it, inserted] = m_returnLabels.try_emplace(&std::get<SSACFG::Call>(_operation.kind).call.get(), 0);
+		auto const [it, inserted] = m_returnLabels.try_emplace(_opId, 0);
 		yulAssert(inserted, "Call sites should be unique.");
 		it->second = m_assembly.newLabelId();
 	}
@@ -244,7 +244,7 @@ void CodeTransform::operator()(SSACFG::OperationId _opId, StackData const& _oper
 		yulAssert(std::holds_alternative<SSACFG::Call>(_operation.kind));
 		yulAssert(
 			returnLabelSlot.isFunctionCallReturnLabel() &&
-			&m_callSites.functionCall(returnLabelSlot.functionCallReturnLabel()) == &std::get<SSACFG::Call>(_operation.kind).call.get()
+			m_callSites.operationId(returnLabelSlot.functionCallReturnLabel()) == _opId
 		);
 	}
 
@@ -262,14 +262,21 @@ void CodeTransform::operator()(SSACFG::OperationId _opId, StackData const& _oper
 	std::visit(util::GenericVisitor{
 		[&](SSACFG::BuiltinCall const& _builtin) {
 			m_assembly.setSourceLocation(opOriginLocation);
-			static_cast<BuiltinFunctionForEVM const&>(_builtin.builtin.get()).generateCode(
-				_builtin.call,
-				m_assembly,
-				m_builtinContext
-			);
+			auto const& builtin = m_cfg.evmDialect.builtin(_builtin.builtin);
+			// build up the call with transient args to handle literal arguments as needed
+			std::vector<Expression> transientArgs;
+			transientArgs.reserve(builtin.literalArguments.size());
+			auto litIt = _builtin.literalArguments.begin();
+			for (size_t i = 0; i < builtin.literalArguments.size(); ++i)
+				if (builtin.literalArgument(i).has_value())
+					transientArgs.emplace_back(*litIt++);
+				else
+					transientArgs.emplace_back(Identifier{});
+			FunctionCall const transient{{}, BuiltinName{{}, _builtin.builtin}, std::move(transientArgs)};
+			builtin.generateCode(transient, m_assembly, m_builtinContext);
 		},
 		[&](SSACFG::Call const& _call) {
-			auto const* returnLabel = util::valueOrNullptr(m_returnLabels, &_call.call.get());
+			auto const* returnLabel = util::valueOrNullptr(m_returnLabels, _opId);
 			// check that if we have a return label, the call can continue
 			yulAssert(!!returnLabel == _call.canContinue);
 			m_assembly.setSourceLocation(opOriginLocation);
@@ -388,8 +395,8 @@ void CodeTransform::operator()(SSACFG::BlockId const& _blockId, SSACFG::BasicBlo
 	auto const& block = m_cfg.block(_blockId);
 	yulAssert(!block.operations.empty(), "Terminated block must have at least one operation.");
 	std::visit(util::GenericVisitor{
-		[](SSACFG::BuiltinCall const& _builtin) {
-			yulAssert(_builtin.builtin.get().controlFlowSideEffects.terminatesOrReverts(), "Last operation of Terminated block must terminate or revert.");
+		[&](SSACFG::BuiltinCall const& _builtin) {
+			yulAssert(m_cfg.evmDialect.builtin(_builtin.builtin).controlFlowSideEffects.terminatesOrReverts(), "Last operation of Terminated block must terminate or revert.");
 		},
 		[](SSACFG::Call const& _call) {
 			yulAssert(!_call.canContinue, "Last operation of Terminated block must be a non-continuable call.");

libyul/backends/evm/ssa/CodeTransform.h

@@ -66,9 +66,9 @@ struct AssemblyCallbacks
 		}
 		case StackSlot::Kind::FunctionCallReturnLabel:
 		{
-			auto const& call = callSites->functionCall(_slot.functionCallReturnLabel());
-			yulAssert(returnLabels->count(&call), "FunctionCallReturnLabel not pre-registered before shuffle.");
-			assembly->appendLabelReference(returnLabels->at(&call));
+			auto const opId = callSites->operationId(_slot.functionCallReturnLabel());
+			yulAssert(returnLabels->count(opId), "FunctionCallReturnLabel not pre-registered before shuffle.");
+			assembly->appendLabelReference(returnLabels->at(opId));
 			return;
 		}
 		case StackSlot::Kind::FunctionReturnLabel:
@@ -86,7 +86,7 @@ struct AssemblyCallbacks
 	SSACFG const* cfg{};
 	AbstractAssembly* assembly{};
 	CallSites const* callSites{};
-	std::map<FunctionCall const*, AbstractAssembly::LabelID> const* returnLabels{};
+	std::map<SSACFG::OperationId, AbstractAssembly::LabelID> const* returnLabels{};
 };
 static_assert(StackManipulationCallbackConcept<AssemblyCallbacks>);
 
@@ -141,7 +141,7 @@ class CodeTransform
 	AssemblyCallbacks m_assemblyCallbacks;
 	StackData m_stackData;
 	Stack<AssemblyCallbacks> m_stack;
-	std::map<FunctionCall const*, AbstractAssembly::LabelID> m_returnLabels;
+	std::map<SSACFG::OperationId, AbstractAssembly::LabelID> m_returnLabels;
 };
 
 }

libyul/backends/evm/ssa/SSACFG.cpp

@@ -111,7 +111,7 @@ class SSACFGDotExporter: public io::DotExporterBase
 					return fmt::format("func{}", _call.graphID);
 				},
 				[&](SSACFG::BuiltinCall const& _call) -> std::string {
-					return _call.builtin.get().name;
+					return m_cfg.evmDialect.builtin(_call.builtin).name;
 				}
 			}, operation.kind);
 			if (!operation.outputs.empty())

libyul/backends/evm/ssa/SSACFG.h

@@ -34,9 +34,6 @@
 #include <libsolutil/Numeric.h>
 
 #include <concepts>
-#include <deque>
-#include <functional>
-#include <list>
 #include <string>
 #include <vector>
 
@@ -70,13 +67,13 @@ class SSACFG
 
 	struct BuiltinCall
 	{
-		std::reference_wrapper<BuiltinFunction const> builtin;
-		std::reference_wrapper<FunctionCall const> call;
+		BuiltinHandle builtin;
+		/// Literal-kind arguments
+		std::vector<Literal> literalArguments;
 	};
 	struct Call
 	{
 		FunctionGraphID graphID;
-		std::reference_wrapper<FunctionCall const> call;
 		bool canContinue;
 	};
 
@@ -281,8 +278,6 @@ class SSACFG
 	bool canContinue = true;
 	std::vector<ValueId> arguments;
 	std::size_t numReturns = 0;
-	// Container for artificial calls generated for switch statements.
-	std::list<FunctionCall> ghostCalls;
 
 	bool isMainGraph() const { return name.empty(); }
 };

libyul/backends/evm/ssa/SSACFGBuilder.cpp

@@ -234,18 +234,10 @@ void SSACFGBuilder::operator()(Switch const& _switch)
 	yulAssert(equalityBuiltinHandle);
 
 	auto makeValueCompare = [&](Case const& _case) {
-		FunctionCall const& ghostCall = m_graph.ghostCalls.emplace_back(FunctionCall{
-			debugDataOf(_case),
-			BuiltinName{{}, *equalityBuiltinHandle},
-			{*_case.value /* skip second argument */ }
-		});
 		auto outputValue = m_graph.newVariable(m_currentBlock);
 		auto opId = m_graph.makeOperation(SSACFG::Operation{
 			{outputValue},
-			SSACFG::BuiltinCall{
-				m_dialect.builtin(*equalityBuiltinHandle),
-				ghostCall
-			},
+			SSACFG::BuiltinCall{*equalityBuiltinHandle, {}},
 			{m_graph.newLiteral(debugDataOf(_case), _case.value->value.value()), expression}
 		}, debugDataOf(_case));
 		currentBlock().operations.emplace_back(opId);
@@ -452,7 +444,15 @@ std::vector<SSACFG::ValueId> SSACFGBuilder::visitFunctionCall(FunctionCall const
 		[&](BuiltinName const& _builtinName)
 		{
 			auto const& builtin = m_dialect.builtin(_builtinName.handle);
-			SSACFG::Operation result{{}, SSACFG::BuiltinCall{builtin, _call}, {}};
+			yulAssert(_call.arguments.size() == builtin.numParameters);
+			std::vector<Literal> literalArguments;
+			for (auto&& [kind, arg]: ranges::views::zip(builtin.literalArguments, _call.arguments))
+				if (kind.has_value())
+				{
+					yulAssert(std::holds_alternative<Literal>(arg));
+					literalArguments.emplace_back(std::get<Literal>(arg));
+				}
+			SSACFG::Operation result{{}, SSACFG::BuiltinCall{_builtinName.handle, std::move(literalArguments)}, {}};
 			for (auto&& [idx, arg]: _call.arguments | ranges::views::enumerate | ranges::views::reverse)
 				if (!builtin.literalArgument(idx).has_value())
 					result.inputs.emplace_back(std::visit(*this, arg));
@@ -470,7 +470,7 @@ std::vector<SSACFG::ValueId> SSACFGBuilder::visitFunctionCall(FunctionCall const
 			canContinue = m_sideEffects.functionSideEffects().at(definition).canContinue;
 			auto const calleeIt = m_functionScopeToID.find(&function);
 			yulAssert(calleeIt != m_functionScopeToID.end(), "Called function has no registered graph id.");
-			SSACFG::Operation result{{}, SSACFG::Call{calleeIt->second, _call, canContinue}, {}};
+			SSACFG::Operation result{{}, SSACFG::Call{calleeIt->second, canContinue}, {}};
 			for (auto const& arg: _call.arguments | ranges::views::reverse)
 				result.inputs.emplace_back(std::visit(*this, arg));
 			for (size_t i = 0; i < function.numReturns; ++i)

libyul/backends/evm/ssa/Stack.h

@@ -27,46 +27,41 @@
 #include <cstdint>
 #include <type_traits>
 
-namespace solidity::yul
-{
-
-struct FunctionCall;
-
-namespace ssa
+namespace solidity::yul::ssa
 {
 
 /// Registry for tracking function call sites.
 ///
-/// Maps FunctionCall AST nodes to unique numeric IDs. These IDs are used
+/// Maps user-function-call operations to unique numeric IDs. These IDs are used
 /// to generate return labels for function calls in the EVM bytecode.
 class CallSites
 {
 public:
 	using CallSiteID = std::uint32_t;
 
-	std::optional<CallSiteID> callSiteID(FunctionCall const* _functionCall) const
+	std::optional<CallSiteID> callSiteID(OperationId _op) const
 	{
-		if (auto const it = ranges::find(m_data, _functionCall); it != m_data.end())
+		if (auto const it = ranges::find(m_data, _op); it != m_data.end())
 			return static_cast<CallSiteID>(std::distance(m_data.begin(), it));
 		return std::nullopt;
 	}
 
-	FunctionCall const& functionCall(CallSiteID _callSite) const
+	OperationId operationId(CallSiteID _callSite) const
 	{
 		yulAssert(_callSite < m_data.size());
-		return *m_data[_callSite];
+		return m_data[_callSite];
 	}
 
-	CallSiteID addCallSite(FunctionCall const* _functionCall)
+	CallSiteID addCallSite(OperationId _op)
 	{
-		if (auto const id = callSiteID(_functionCall))
+		if (auto const id = callSiteID(_op))
 			return *id;
-		yulAssert(_functionCall);
-		m_data.emplace_back(_functionCall);
+		yulAssert(_op.hasValue());
+		m_data.emplace_back(_op);
 		return static_cast<CallSiteID>(m_data.size() - 1);
 	}
 private:
-	std::vector<FunctionCall const*> m_data;
+	std::vector<OperationId> m_data;
 };
 
 /// A discriminated union corresponding to a single EVM stack slot.
@@ -309,4 +304,3 @@ class Stack
 };
 
 }
-}

libyul/backends/evm/ssa/StackLayoutGenerator.cpp

@@ -239,7 +239,7 @@ void StackLayoutGenerator::visitBlock(SSACFG::BlockId const& _blockId)
 		if (auto const* call = std::get_if<SSACFG::Call>(&operation.kind))
 			if (call->canContinue)
 			{
-				auto const callSiteID = m_callSites.callSiteID(&call->call.get());
+				auto const callSiteID = m_callSites.callSiteID(block.operations[operationIndex]);
 				yulAssert(callSiteID.has_value());
 				requiredStackTop.emplace_back(Slot::makeFunctionCallReturnLabel(*callSiteID));
 			}

libyul/backends/evm/ssa/StackUtils.cpp

@@ -206,7 +206,7 @@ CallSites solidity::yul::ssa::gatherCallSites(SSACFG const& _cfg)
 		for (auto const opId: block.operations)
 			if (auto const* call = std::get_if<SSACFG::Call>(&_cfg.operation(opId).kind))
 				if (call->canContinue)
-					result.addCallSite(&call->call.get());
+					result.addCallSite(opId);
 	}
 	return result;
 }

libyul/backends/evm/ssa/io/JSONExporter.cpp

@@ -54,26 +54,13 @@ Json toJson(Json& _ret, SSACFG const& _cfg, SSACFG::Operation const& _operation,
 		[&](SSACFG::BuiltinCall const& _call) {
 			_ret["type"] = "BuiltinCall";
 			Json builtinArgsJson = Json::array();
-			auto const& builtin = _call.builtin.get();
-			if (!builtin.literalArguments.empty())
-			{
-				auto const& functionCallArgs = _call.call.get().arguments;
-				for (size_t i = 0; i < builtin.literalArguments.size(); ++i)
-				{
-					std::optional<LiteralKind> const& argument = builtin.literalArguments[i];
-					if (argument.has_value() && i < functionCallArgs.size())
-					{
-						// The function call argument at index i must be a literal if builtin.literalArguments[i] is not nullopt
-						yulAssert(std::holds_alternative<Literal>(functionCallArgs[i]));
-						builtinArgsJson.push_back(formatLiteral(std::get<Literal>(functionCallArgs[i])));
-					}
-				}
-			}
+			for (auto const& literal: _call.literalArguments)
+				builtinArgsJson.push_back(formatLiteral(literal));
 
 			if (!builtinArgsJson.empty())
 				opJson["literalArgs"] = builtinArgsJson;
 
-			opJson["op"] = _call.builtin.get().name;
+			opJson["op"] = _cfg.evmDialect.builtin(_call.builtin).name;
 		},
 	}, _operation.kind);
 

test/libyul/ssa/StackLayoutGeneratorTest.cpp

@@ -92,7 +92,7 @@ class StackLayoutDotExporter: public io::DotExporterBase
 					_out << escapeLabel(m_controlFlow.functionGraph(_call.graphID)->name);
 				},
 				[&](SSACFG::BuiltinCall const& _call) {
-					_out << escapeLabel(_call.builtin.get().name);
+					_out << escapeLabel(m_cfg.evmDialect.builtin(_call.builtin).name);
 				}
 			}, operation.kind);
 			_out << "\\l\\\n";