Easy review fixes.

docs/yul.rst

@@ -958,7 +958,7 @@ memoryguard
 This function is available in the EVM dialect with objects. The caller of
 ``let ptr := memoryguard(size)`` (where ``size`` has to be a literal number)
 promises that they only use memory in either the range ``[0, size)`` or the
-unbounded range above ``ptr``.
+unbounded range starting at ``ptr``.
 
 Since the presence of a ``memoryguard`` call indicates that all memory access
 adheres to this restriction, it allows the optimizer to perform additional

libsolidity/codegen/ir/IRGenerator.cpp

@@ -142,7 +142,10 @@ string IRGenerator::generate(
 	InternalDispatchMap internalDispatchMap = generateInternalDispatchFunctions();
 	t("functions", m_context.functionCollector().requestedFunctions());
 	t("subObjects", subObjectSources(m_context.subObjectsCreated()));
-	t("memoryInitCreation", memoryInit(!m_context.inlineAssemblySeen()));
+
+	// This has to be called only after all other code generation for the creation object is complete.
+	bool creationInvolvesAssembly = m_context.inlineAssemblySeen();
+	t("memoryInitCreation", memoryInit(!creationInvolvesAssembly));
 
 	resetContext(_contract);
 
@@ -158,7 +161,10 @@ string IRGenerator::generate(
 	generateInternalDispatchFunctions();
 	t("runtimeFunctions", m_context.functionCollector().requestedFunctions());
 	t("runtimeSubObjects", subObjectSources(m_context.subObjectsCreated()));
-	t("memoryInitRuntime", memoryInit(!m_context.inlineAssemblySeen()));
+
+	// This has to be called only after all other code generation for the runtime object is complete.
+	bool runtimeInvolvesAssembly = m_context.inlineAssemblySeen();
+	t("memoryInitRuntime", memoryInit(!runtimeInvolvesAssembly));
 	return t.render();
 }
 

libyul/optimiser/StackLimitEvader.cpp

@@ -35,31 +35,33 @@ using namespace solidity::yul;
 
 namespace
 {
-// Walks the call graph using a Depth-First-Search assigning memory offsets to variables.
-// - The leaves of the call graph will get the lowest offsets, increasing towards the root.
-// - ``nextAvailableSlot`` maps a function to the next available slot that can be used by another
-//   function that calls it.
-// - For each function starting from the root of the call graph:
-//   - Visit all children that are not already visited.
-//   - Determine the maximum value ``n`` of the values of ``nextAvailableSlot`` among the children.
-//   - If the function itself contains variables that need memory slots, but is contained in a cycle,
-//     abort the process as failure.
-//   - If not, assign each variable its slot starting from ``n`` (incrementing it).
-//   - Assign ``n`` to ``nextAvailableSlot`` of the function.
+/**
+ * Walks the call graph using a Depth-First-Search assigning memory slots to variables.
+ * - The leaves of the call graph will get the lowest slot, increasing towards the root.
+ * - ``slotsRequiredForFunction`` maps a function to the number of slots it requires (which is also the
+ *   next available slot that can be used by another function that calls this function).
+ * - For each function starting from the root of the call graph:
+ * - Visit all children that are not already visited.
+ * - Determine the maximum value ``n`` of the values of ``slotsRequiredForFunction`` among the children.
+ * - If the function itself contains variables that need memory slots, but is contained in a cycle,
+ *   abort the process as failure.
+ * - If not, assign each variable its slot starting from ``n`` (incrementing it).
+ * - Assign ``n`` to ``slotsRequiredForFunction`` of the function.
+ */
 struct MemoryOffsetAllocator
 {
 	uint64_t run(YulString _function = YulString{})
 	{
-		if (nextAvailableSlot.count(_function))
-			return nextAvailableSlot[_function];
+		if (slotsRequiredForFunction.count(_function))
+			return slotsRequiredForFunction[_function];
 
 		// Assign to zero early to guard against recursive calls.
-		nextAvailableSlot[_function] = 0;
+		slotsRequiredForFunction[_function] = 0;
 
-		uint64_t nextSlot = 0;
+		uint64_t requiredSlots = 0;
 		if (callGraph.count(_function))
 			for (YulString child: callGraph.at(_function))
-				nextSlot = std::max(run(child), nextSlot);
+				requiredSlots = std::max(run(child), requiredSlots);
 
 		if (unreachableVariables.count(_function))
 		{
@@ -71,17 +73,17 @@ struct MemoryOffsetAllocator
 					// TODO: Too many function arguments or return parameters.
 				}
 				else
-					assignedSlots[variable] = nextSlot++;
+					assignedSlots[variable] = requiredSlots++;
 		}
 
-		return nextAvailableSlot[_function] = nextSlot;
+		return slotsRequiredForFunction[_function] = requiredSlots;
 	}
 
 	map<YulString, set<YulString>> const& unreachableVariables;
 	map<YulString, set<YulString>> const& callGraph;
 
 	map<YulString, map<YulString, uint64_t>> slotAllocations{};
-	map<YulString, uint64_t> nextAvailableSlot{};
+	map<YulString, uint64_t> slotsRequiredForFunction{};
 };
 
 u256 literalArgumentValue(FunctionCall const& _call)
@@ -116,8 +118,8 @@ void StackLimitEvader::run(
 
 	// Make sure all calls to ``memoryguard`` we found have the same value as argument (otherwise, abort).
 	u256 reservedMemory = literalArgumentValue(*memoryGuardCalls.front());
-	for (FunctionCall const* getFreeMemoryStartCall: memoryGuardCalls)
-		if (reservedMemory != literalArgumentValue(*getFreeMemoryStartCall))
+	for (FunctionCall const* memoryGuardCall: memoryGuardCalls)
+		if (reservedMemory != literalArgumentValue(*memoryGuardCall))
 			return;
 
 	CallGraph callGraph = CallGraphGenerator::callGraph(*_object.code);
@@ -130,13 +132,14 @@ void StackLimitEvader::run(
 	MemoryOffsetAllocator memoryOffsetAllocator{_unreachableVariables, callGraph.functionCalls};
 	uint64_t requiredSlots = memoryOffsetAllocator.run();
 
-	StackToMemoryMover{_context, reservedMemory, memoryOffsetAllocator.slotAllocations}(*_object.code);
+	StackToMemoryMover::run(_context, reservedMemory, memoryOffsetAllocator.slotAllocations, *_object.code);
+
+	yulAssert(requiredSlots < std::numeric_limits<uint64_t>::max() / 32, "");
 	reservedMemory += 32 * requiredSlots;
-	YulString reservedMemoryString{util::toCompactHexWithPrefix(reservedMemory)};
-	for (FunctionCall* memoryGuardCall: memoryGuardCalls)
+	for (FunctionCall* memoryGuardCall: FunctionCallFinder::run(*_object.code, "memoryguard"_yulstring))
 	{
 		Literal* literal = std::get_if<Literal>(&memoryGuardCall->arguments.front());
 		yulAssert(literal && literal->kind == LiteralKind::Number, "");
-		literal->value = reservedMemoryString;
+		literal->value = YulString{util::toCompactHexWithPrefix(reservedMemory)};
 	}
 }

libyul/optimiser/StackToMemoryMover.cpp

@@ -29,14 +29,14 @@ using namespace solidity::yul;
 
 namespace
 {
-void appendMemoryStore(
-	vector<Statement>& _statements,
+vector<Statement> generateMemoryStore(
 	langutil::SourceLocation const& _loc,
 	YulString _mpos,
 	Expression _value
 )
 {
-	_statements.emplace_back(ExpressionStatement{_loc, FunctionCall{
+	vector<Statement> result;
+	result.emplace_back(ExpressionStatement{_loc, FunctionCall{
 		_loc,
 		Identifier{_loc, "mstore"_yulstring},
 		{
@@ -44,9 +44,21 @@ void appendMemoryStore(
 			std::move(_value)
 		}
 	}});
+	return result;
 }
 }
 
+void StackToMemoryMover::run(
+	OptimiserStepContext& _context,
+	u256 _reservedMemory,
+	map<YulString, map<YulString, uint64_t>> const& _memorySlots,
+	Block& _block
+)
+{
+	StackToMemoryMover stackToMemoryMover(_context, _reservedMemory, _memorySlots);
+	stackToMemoryMover(_block);
+}
+
 StackToMemoryMover::StackToMemoryMover(
 	OptimiserStepContext& _context,
 	u256 _reservedMemory,
@@ -66,22 +78,20 @@ StackToMemoryMover::StackToMemoryMover(
 
 void StackToMemoryMover::operator()(FunctionDefinition& _functionDefinition)
 {
-	map<YulString, uint64_t> const* saved = m_currentFunctionMemorySlots;
 	if (m_memorySlots.count(_functionDefinition.name))
 	{
+		map<YulString, uint64_t> const* saved = m_currentFunctionMemorySlots;
 		m_currentFunctionMemorySlots = &m_memorySlots.at(_functionDefinition.name);
 		for (TypedName const& param: _functionDefinition.parameters + _functionDefinition.returnVariables)
 			if (m_currentFunctionMemorySlots->count(param.name))
 			{
 				// TODO: we cannot handle function parameters yet.
-				m_currentFunctionMemorySlots = nullptr;
-				break;
+				m_currentFunctionMemorySlots = saved;
+				return;
 			}
+		ASTModifier::operator()(_functionDefinition);
+		m_currentFunctionMemorySlots = saved;
 	}
-	else
-		m_currentFunctionMemorySlots = nullptr;
-	ASTModifier::operator()(_functionDefinition);
-	m_currentFunctionMemorySlots = saved;
 }
 
 void StackToMemoryMover::operator()(Block& _block)
@@ -103,16 +113,11 @@ void StackToMemoryMover::operator()(Block& _block)
 		std::unique_ptr<Expression> _value
 	) -> std::vector<Statement> {
 		if (_variables.size() == 1)
-		{
-			std::vector<Statement> result;
-			appendMemoryStore(
-				result,
+			return generateMemoryStore(
 				_loc,
 				memoryOffset(_variables.front().name),
 				_value ? *std::move(_value) : Literal{_loc, LiteralKind::Number, "0"_yulstring, {}}
 			);
-			return result;
-		}
 
 		VariableDeclaration tempDecl{_loc, {}, std::move(_value)};
 		vector<Statement> memoryAssignments;
@@ -123,7 +128,7 @@ void StackToMemoryMover::operator()(Block& _block)
 			tempDecl.variables.emplace_back(TypedName{var.location, tempVarName, {}});
 
 			if (m_currentFunctionMemorySlots->count(var.name))
-				appendMemoryStore(memoryAssignments, _loc, memoryOffset(var.name), Identifier{_loc, tempVarName});
+				memoryAssignments += generateMemoryStore(_loc, memoryOffset(var.name), Identifier{_loc, tempVarName});
 			else if constexpr (std::is_same_v<std::decay_t<decltype(var)>, Identifier>)
 				variableAssignments.emplace_back(Assignment{
 					_loc, { Identifier{var.location, var.name} },
@@ -186,10 +191,10 @@ void StackToMemoryMover::visit(Expression& _expression)
 	)
 	{
 		langutil::SourceLocation loc = identifier->location;
-		_expression = FunctionCall {
+		_expression = FunctionCall{
 			loc,
 			Identifier{loc, "mload"_yulstring}, {
-				Literal {
+				Literal{
 					loc,
 					LiteralKind::Number,
 					memoryOffset(identifier->name),

libyul/optimiser/StackToMemoryMover.h

@@ -73,23 +73,37 @@ namespace solidity::yul
  * If a visited function has arguments or return parameters that are contained in the map,
  * the entire function is skipped (no local variables in the function will be moved at all).
  *
- * Prerequisite: Disambiguator, ForLoopInitRewriter.
+ * Prerequisite: Disambiguator, ForLoopInitRewriter, FunctionHoister.
  */
 class StackToMemoryMover: ASTModifier
 {
 public:
-	StackToMemoryMover(
+	/**
+	 * Runs the stack to memory mover.
+	 * @param _reservedMemory Is the amount of previously reserved memory,
+	 *                        i.e. the lowest memory offset to which variables can be moved.
+	 * @param _memorySlots A map from variables to a slot in memory. The offset to which a variables will be moved
+	 *                       is given by _reservedMemory plus 32 times its entry in @a _memorySlots.
+	 */
+	static void run(
 		OptimiserStepContext& _context,
 		u256 _reservedMemory,
-		std::map<YulString, std::map<YulString, uint64_t>> const& _memoryOffsets
+		std::map<YulString, std::map<YulString, uint64_t>> const& _memorySlots,
+		Block& _block
 	);
-
 	using ASTModifier::operator();
 
 	void operator()(FunctionDefinition& _functionDefinition) override;
 	void operator()(Block& _block) override;
 	void visit(Expression& _expression) override;
 private:
+	StackToMemoryMover(
+		OptimiserStepContext& _context,
+		u256 _reservedMemory,
+		std::map<YulString, std::map<YulString, uint64_t>> const& _memorySlots
+	);
+
+	/// @returns a YulString containing the memory offset to be assigned to @a _variable as number literal.
 	YulString memoryOffset(YulString _variable);
 	u256 m_reservedMemory;
 	std::map<YulString, std::map<YulString, uint64_t>> const& m_memorySlots;