Merge pull request #12759 from ethereum/evmCodeTransformAvoidPops

Avoid unnecessary pops in evm code transform.

Changelog.md

@@ -16,6 +16,7 @@ Compiler Features:
  * JSON-AST: Added selector field for errors and events.
  * LSP: Implements goto-definition.
  * Peephole Optimizer: Optimize comparisons in front of conditional jumps and conditional jumps across a single unconditional jump.
+ * Yul EVM Code Transform: Avoid unnecessary ``pop``s on terminating control flow.
  * Yul Optimizer: Remove ``sstore`` and ``mstore`` operations that are never read from.
 
 

libevmasm/PeepholeOptimiser.cpp

@@ -117,6 +117,35 @@ struct OpPop: SimplePeepholeOptimizerMethod<OpPop>
 	}
 };
 
+struct OpStop: SimplePeepholeOptimizerMethod<OpStop>
+{
+	static bool applySimple(
+		AssemblyItem const& _op,
+		AssemblyItem const& _stop,
+		std::back_insert_iterator<AssemblyItems> _out
+	)
+	{
+		if (_stop == Instruction::STOP)
+		{
+			if (_op.type() == Operation)
+			{
+				Instruction instr = _op.instruction();
+				if (!instructionInfo(instr).sideEffects)
+				{
+					*_out = {Instruction::STOP, _op.location()};
+					return true;
+				}
+			}
+			else if (_op.type() == Push)
+			{
+				*_out = {Instruction::STOP, _op.location()};
+				return true;
+			}
+		}
+		return false;
+	}
+};
+
 struct DoubleSwap: SimplePeepholeOptimizerMethod<DoubleSwap>
 {
 	static size_t applySimple(AssemblyItem const& _s1, AssemblyItem const& _s2, std::back_insert_iterator<AssemblyItems>)
@@ -430,7 +459,7 @@ bool PeepholeOptimiser::optimise()
 	while (state.i < m_items.size())
 		applyMethods(
 			state,
-			PushPop(), OpPop(), DoublePush(), DoubleSwap(), CommutativeSwap(), SwapComparison(),
+			PushPop(), OpPop(), OpStop(), DoublePush(), DoubleSwap(), CommutativeSwap(), SwapComparison(),
 			DupSwap(), IsZeroIsZeroJumpI(), EqIsZeroJumpI(), DoubleJump(), JumpToNext(), UnreachableCode(),
 			TagConjunctions(), TruthyAnd(), Identity()
 		);

libyul/backends/evm/ControlFlowGraph.h

@@ -195,6 +195,14 @@ struct CFG
 		std::shared_ptr<DebugData const> debugData;
 		std::vector<BasicBlock*> entries;
 		std::vector<Operation> operations;
+		/// True, if the block is the beginning of a disconnected subgraph. That is, if no block that is reachable
+		/// from this block is an ancestor of this block. In other words, this is true, if this block is the target
+		/// of a cut-edge/bridge in the CFG or if the block itself terminates.
+		bool isStartOfSubGraph = false;
+		/// True, if there is a path from this block to a function return.
+		bool needsCleanStack = false;
+		/// If the block starts a sub-graph and does not lead to a function return, we are free to add junk to it.
+		bool allowsJunk() const { return isStartOfSubGraph && !needsCleanStack; }
 		std::variant<MainExit, Jump, ConditionalJump, FunctionReturn, Terminated> exit = MainExit{};
 	};
 
@@ -205,6 +213,7 @@ struct CFG
 		BasicBlock* entry = nullptr;
 		std::vector<VariableSlot> parameters;
 		std::vector<VariableSlot> returnVariables;
+		std::vector<BasicBlock*> exits;
 	};
 
 	/// The main entry point, i.e. the start of the outermost Yul block.

libyul/backends/evm/ControlFlowGraphBuilder.cpp

@@ -48,7 +48,7 @@ using namespace std;
 
 namespace
 {
-// Removes edges to blocks that are not reachable.
+/// Removes edges to blocks that are not reachable.
 void cleanUnreachable(CFG& _cfg)
 {
 	// Determine which blocks are reachable from the entry.
@@ -77,7 +77,8 @@ void cleanUnreachable(CFG& _cfg)
 			return !reachabilityCheck.visited.count(entry);
 		});
 }
-// Sets the ``recursive`` member to ``true`` for all recursive function calls.
+
+/// Sets the ``recursive`` member to ``true`` for all recursive function calls.
 void markRecursiveCalls(CFG& _cfg)
 {
 	map<CFG::BasicBlock*, vector<CFG::FunctionCall*>> callsPerBlock;
@@ -124,6 +125,84 @@ void markRecursiveCalls(CFG& _cfg)
 			});
 		}
 }
+
+/// Marks each cut-vertex in the CFG, i.e. each block that begins a disconnected sub-graph of the CFG.
+/// Entering such a block means that control flow will never return to a previously visited block.
+void markStartsOfSubGraphs(CFG& _cfg)
+{
+	vector<CFG::BasicBlock*> entries;
+	entries.emplace_back(_cfg.entry);
+	for (auto&& functionInfo: _cfg.functionInfo | ranges::views::values)
+		entries.emplace_back(functionInfo.entry);
+	for (auto& entry: entries)
+	{
+		/**
+		 * Detect bridges following Algorithm 1 in https://arxiv.org/pdf/2108.07346.pdf
+		 * and mark the bridge targets as starts of sub-graphs.
+		 */
+		set<CFG::BasicBlock*> visited;
+		map<CFG::BasicBlock*, size_t> disc;
+		map<CFG::BasicBlock*, size_t> low;
+		map<CFG::BasicBlock*, CFG::BasicBlock*> parent;
+		size_t time = 0;
+		auto dfs = [&](CFG::BasicBlock* _u, auto _recurse) -> void {
+			visited.insert(_u);
+			disc[_u] = low[_u] = time;
+			time++;
+
+			vector<CFG::BasicBlock*> children = _u->entries;
+			visit(util::GenericVisitor{
+				[&](CFG::BasicBlock::Jump const& _jump) {
+					children.emplace_back(_jump.target);
+				},
+				[&](CFG::BasicBlock::ConditionalJump const& _jump) {
+					children.emplace_back(_jump.zero);
+					children.emplace_back(_jump.nonZero);
+				},
+				[&](CFG::BasicBlock::FunctionReturn const&) {},
+				[&](CFG::BasicBlock::Terminated const&) { _u->isStartOfSubGraph = true; },
+				[&](CFG::BasicBlock::MainExit const&) { _u->isStartOfSubGraph = true; }
+			}, _u->exit);
+			yulAssert(!util::contains(children, _u));
+
+			for (CFG::BasicBlock* v: children)
+				if (!visited.count(v))
+				{
+					parent[v] = _u;
+					_recurse(v, _recurse);
+					low[_u] = min(low[_u], low[v]);
+					if (low[v] > disc[_u])
+					{
+						// _u <-> v is a cut edge in the undirected graph
+						bool edgeVtoU = util::contains(_u->entries, v);
+						bool edgeUtoV = util::contains(v->entries, _u);
+						if (edgeVtoU && !edgeUtoV)
+							// Cut edge v -> _u
+							_u->isStartOfSubGraph = true;
+						else if (edgeUtoV && !edgeVtoU)
+							// Cut edge _u -> v
+							v->isStartOfSubGraph = true;
+					}
+				}
+				else if (v != parent[_u])
+					low[_u] = min(low[_u], disc[v]);
+		};
+		dfs(entry, dfs);
+	}
+}
+
+/// Marks each block that needs to maintain a clean stack. That is each block that has an outgoing
+/// path to a function return.
+void markNeedsCleanStack(CFG& _cfg)
+{
+	for (auto& functionInfo: _cfg.functionInfo | ranges::views::values)
+		for (CFG::BasicBlock* exit: functionInfo.exits)
+			util::BreadthFirstSearch<CFG::BasicBlock*>{{exit}}.run([&](CFG::BasicBlock* _block, auto _addChild) {
+				_block->needsCleanStack = true;
+				for (CFG::BasicBlock* entry: _block->entries)
+					_addChild(entry);
+			});
+}
 }
 
 std::unique_ptr<CFG> ControlFlowGraphBuilder::build(
@@ -141,6 +220,8 @@ std::unique_ptr<CFG> ControlFlowGraphBuilder::build(
 
 	cleanUnreachable(*result);
 	markRecursiveCalls(*result);
+	markStartsOfSubGraphs(*result);
+	markNeedsCleanStack(*result);
 
 	// TODO: It might be worthwhile to run some further simplifications on the graph itself here.
 	// E.g. if there is a jump to a node that has the jumping node as its only entry, the nodes can be fused, etc.
@@ -379,6 +460,7 @@ void ControlFlowGraphBuilder::operator()(Leave const& leave_)
 {
 	yulAssert(m_currentFunction.has_value(), "");
 	m_currentBlock->exit = CFG::BasicBlock::FunctionReturn{debugDataOf(leave_), *m_currentFunction};
+	(*m_currentFunction)->exits.emplace_back(m_currentBlock);
 	m_currentBlock = &m_graph.makeBlock(debugDataOf(*m_currentBlock));
 }
 
@@ -395,6 +477,7 @@ void ControlFlowGraphBuilder::operator()(FunctionDefinition const& _function)
 	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};
 }
 
@@ -423,7 +506,8 @@ void ControlFlowGraphBuilder::registerFunction(FunctionDefinition const& _functi
 				std::get<Scope::Variable>(virtualFunctionScope->identifiers.at(_retVar.name)),
 				_retVar.debugData
 			};
-		}) | ranges::to<vector>
+		}) | ranges::to<vector>,
+		{}
 	})).second;
 	yulAssert(inserted);
 }

libyul/backends/evm/StackLayoutGenerator.cpp

@@ -23,6 +23,8 @@
 
 #include <libyul/backends/evm/StackHelpers.h>
 
+#include <libevmasm/GasMeter.h>
+
 #include <libsolutil/Algorithms.h>
 #include <libsolutil/cxx20.h>
 #include <libsolutil/Visitor.h>
@@ -400,6 +402,7 @@ void StackLayoutGenerator::processEntryPoint(CFG::BasicBlock const& _entry)
 	}
 
 	stitchConditionalJumps(_entry);
+	fillInJunk(_entry);
 }
 
 optional<Stack> StackLayoutGenerator::getExitLayoutOrStageDependencies(
@@ -703,3 +706,110 @@ Stack StackLayoutGenerator::compressStack(Stack _stack)
 	while (firstDupOffset);
 	return _stack;
 }
+
+void StackLayoutGenerator::fillInJunk(CFG::BasicBlock const& _block)
+{
+	/// Recursively adds junk to the subgraph starting on @a _entry.
+	/// Since it is only called on cut-vertices, the full subgraph retains proper stack balance.
+	auto addJunkRecursive = [&](CFG::BasicBlock const* _entry, size_t _numJunk) {
+		util::BreadthFirstSearch<CFG::BasicBlock const*> breadthFirstSearch{{_entry}};
+		breadthFirstSearch.run([&](CFG::BasicBlock const* _block, auto _addChild) {
+			auto& blockInfo = m_layout.blockInfos.at(_block);
+			blockInfo.entryLayout = Stack{_numJunk, JunkSlot{}} + move(blockInfo.entryLayout);
+			for (auto const& operation: _block->operations)
+			{
+				auto& operationEntryLayout = m_layout.operationEntryLayout.at(&operation);
+				operationEntryLayout = Stack{_numJunk, JunkSlot{}} + move(operationEntryLayout);
+			}
+			blockInfo.exitLayout = Stack{_numJunk, JunkSlot{}} + move(blockInfo.exitLayout);
+
+			std::visit(util::GenericVisitor{
+				[&](CFG::BasicBlock::MainExit const&) {},
+				[&](CFG::BasicBlock::Jump const& _jump)
+				{
+					_addChild(_jump.target);
+				},
+				[&](CFG::BasicBlock::ConditionalJump const& _conditionalJump)
+				{
+					_addChild(_conditionalJump.zero);
+					_addChild(_conditionalJump.nonZero);
+				},
+				[&](CFG::BasicBlock::FunctionReturn const&) { yulAssert(false); },
+				[&](CFG::BasicBlock::Terminated const&) {},
+			}, _block->exit);
+		});
+	};
+	/// @returns the number of operations required to transform @a _source to @a _target.
+	auto evaluateTransform = [](Stack _source, Stack const& _target) -> size_t {
+		size_t opGas = 0;
+		auto swap = [&](unsigned _swapDepth)
+		{
+			if (_swapDepth > 16)
+				opGas += 1000;
+			else
+				opGas += evmasm::GasMeter::runGas(evmasm::swapInstruction(_swapDepth));
+		};
+		auto dupOrPush = [&](StackSlot const& _slot)
+		{
+			if (canBeFreelyGenerated(_slot))
+				opGas += evmasm::GasMeter::runGas(evmasm::pushInstruction(32));
+			else
+			{
+				auto depth = util::findOffset(_source | ranges::views::reverse, _slot);
+				yulAssert(depth);
+				if (*depth < 16)
+					opGas += evmasm::GasMeter::runGas(evmasm::dupInstruction(static_cast<unsigned>(*depth + 1)));
+				else
+					opGas += 1000;
+			}
+		};
+		auto pop = [&]() { opGas += evmasm::GasMeter::runGas(evmasm::Instruction::POP); };
+		createStackLayout(_source, _target, swap, dupOrPush, pop);
+		return opGas;
+	};
+	/// Traverses the CFG and at each block that allows junk, i.e. that is a cut-vertex that never leads to a function
+	/// return, checks if adding junk reduces the shuffling cost upon entering and if so recursively adds junk
+	/// to the spanned subgraph.
+	util::BreadthFirstSearch<CFG::BasicBlock const*>{{&_block}}.run([&](CFG::BasicBlock const* _block, auto _addChild) {
+		std::visit(util::GenericVisitor{
+			[&](CFG::BasicBlock::MainExit const&) {},
+			[&](CFG::BasicBlock::Jump const& _jump)
+			{
+				_addChild(_jump.target);
+			},
+			[&](CFG::BasicBlock::ConditionalJump const& _conditionalJump)
+			{
+				for (CFG::BasicBlock* exit: {_conditionalJump.zero, _conditionalJump.nonZero})
+					if (exit->allowsJunk())
+					{
+						auto& blockInfo = m_layout.blockInfos.at(exit);
+						Stack entryLayout = blockInfo.entryLayout;
+						Stack nextLayout = exit->operations.empty() ? blockInfo.exitLayout : m_layout.operationEntryLayout.at(&exit->operations.front());
+
+						size_t bestCost = evaluateTransform(entryLayout, nextLayout);
+						size_t bestNumJunk = 0;
+						size_t maxJunk = entryLayout.size();
+						for (size_t numJunk = 1; numJunk <= maxJunk; ++numJunk)
+						{
+							size_t cost = evaluateTransform(entryLayout, Stack{numJunk, JunkSlot{}} + nextLayout);
+							if (cost < bestCost)
+							{
+								bestCost = cost;
+								bestNumJunk = numJunk;
+							}
+						}
+
+						if (bestNumJunk > 0)
+						{
+							addJunkRecursive(exit, bestNumJunk);
+							blockInfo.entryLayout = entryLayout;
+						}
+					}
+				_addChild(_conditionalJump.zero);
+				_addChild(_conditionalJump.nonZero);
+			},
+			[&](CFG::BasicBlock::FunctionReturn const&) {},
+			[&](CFG::BasicBlock::Terminated const&) {},
+		}, _block->exit);
+	});
+}

libyul/backends/evm/StackLayoutGenerator.h

@@ -111,6 +111,9 @@ class StackLayoutGenerator
 	/// stack @a _stack.
 	static Stack compressStack(Stack _stack);
 
+	//// Fills in junk when entering branches that do not need a clean stack in case the result is cheaper.
+	void fillInJunk(CFG::BasicBlock const& _block);
+
 	StackLayout& m_layout;
 };
 

test/cmdlineTests/dup_opt_peephole/output

@@ -46,8 +46,6 @@ sub_0: assembly {
         /* "dup_opt_peephole/input.sol":150:162  sstore(0, x) */
       sstore
         /* "dup_opt_peephole/input.sol":107:166  {... */
-      pop
-        /* "dup_opt_peephole/input.sol":60:171  contract C {... */
       stop
 
     auxdata: <AUXDATA REMOVED>

test/cmdlineTests/function_debug_info_via_yul/output

@@ -13,7 +13,7 @@
         },
         "calldata_array_index_access_uint256_dyn_calldata":
         {
-          "entryPoint": 152,
+          "entryPoint": 145,
           "parameterSlots": 2,
           "returnSlots": 1
         }