Merge pull request #15702 from ethereum/liveness_fixes

clonker · web-flow · commit c6ed825a6ebf · 2025-01-09T18:08:52.000+01:00
Small fixes for liveness
diff --git a/libyul/backends/evm/ControlFlow.cpp b/libyul/backends/evm/ControlFlow.cpp
@@ -26,3 +26,8 @@ ControlFlowLiveness::ControlFlowLiveness(ControlFlow const& _controlFlow):
 	mainLiveness(std::make_unique<SSACFGLiveness>(*_controlFlow.mainGraph)),
 	functionLiveness(_controlFlow.functionGraphs | ranges::views::transform([](auto const& _cfg) { return std::make_unique<SSACFGLiveness>(*_cfg); }) | ranges::to<std::vector>)
 { }
+
+std::string ControlFlowLiveness::toDot() const
+{
+	return controlFlow.get().toDot(this);
+}
diff --git a/libyul/backends/evm/ControlFlow.h b/libyul/backends/evm/ControlFlow.h
@@ -34,6 +34,8 @@ struct ControlFlowLiveness{
 	std::reference_wrapper<ControlFlow const> controlFlow;
 	std::unique_ptr<SSACFGLiveness> mainLiveness;
 	std::vector<std::unique_ptr<SSACFGLiveness>> functionLiveness;
+
+	std::string toDot() const;
 };
 
 struct ControlFlow
diff --git a/libyul/backends/evm/SSACFGLiveness.cpp b/libyul/backends/evm/SSACFGLiveness.cpp
@@ -37,6 +37,29 @@ constexpr auto literalsFilter(SSACFG const& _cfg)
 }
 }
 
+std::set<SSACFG::ValueId> SSACFGLiveness::blockExitValues(SSACFG::BlockId const& _blockId) const
+{
+	std::set<SSACFG::ValueId> result;
+	util::GenericVisitor exitVisitor {
+		[](SSACFG::BasicBlock::MainExit const&) {},
+		[&](SSACFG::BasicBlock::FunctionReturn const& _functionReturn) {
+			result += _functionReturn.returnValues | ranges::views::filter(literalsFilter(m_cfg));
+		},
+		[&](SSACFG::BasicBlock::JumpTable const& _jt) {
+			if (literalsFilter(m_cfg)(_jt.value))
+				result.emplace(_jt.value);
+		},
+		[](SSACFG::BasicBlock::Jump const&) {},
+		[&](SSACFG::BasicBlock::ConditionalJump const& _conditionalJump) {
+			if (literalsFilter(m_cfg)(_conditionalJump.condition))
+				result.emplace(_conditionalJump.condition);
+		},
+		[](SSACFG::BasicBlock::Terminated const&) {}
+	};
+	std::visit(exitVisitor, m_cfg.block(_blockId).exit);
+	return result;
+}
+
 SSACFGLiveness::SSACFGLiveness(SSACFG const& _cfg):
 	m_cfg(_cfg),
 	m_topologicalSort(_cfg),
@@ -70,12 +93,7 @@ void SSACFGLiveness::runDagDfs()
 				{
 					auto const& info = m_cfg.valueInfo(phi);
 					yulAssert(std::holds_alternative<SSACFG::PhiValue>(info), "value info of phi wasn't PhiValue");
-					auto const& entries = m_cfg.block(std::get<SSACFG::PhiValue>(info).block).entries;
-					// this is getting the argument index of the phi function corresponding to the path going
-					// through "blockId", ie, the currently handled block
-					auto const it = entries.find(blockId);
-					yulAssert(it != entries.end());
-					auto const argIndex = static_cast<size_t>(std::distance(entries.begin(), it));
+					auto const argIndex = m_cfg.phiArgumentIndex(blockId, _successor);
 					yulAssert(argIndex < std::get<SSACFG::PhiValue>(info).arguments.size());
 					auto const arg = std::get<SSACFG::PhiValue>(info).arguments.at(argIndex);
 					if (!std::holds_alternative<SSACFG::LiteralValue>(m_cfg.valueInfo(arg)))
@@ -103,23 +121,7 @@ void SSACFGLiveness::runDagDfs()
 		// for each program point p in B, backwards, do:
 		{
 			// add value ids to the live set that are used in exit blocks
-			util::GenericVisitor exitVisitor {
-				[](SSACFG::BasicBlock::MainExit const&) {},
-				[&](SSACFG::BasicBlock::FunctionReturn const& _functionReturn) {
-					live += _functionReturn.returnValues | ranges::views::filter(literalsFilter(m_cfg));
-				},
-				[&](SSACFG::BasicBlock::JumpTable const& _jt) {
-					if (literalsFilter(m_cfg)(_jt.value))
-						live.emplace(_jt.value);
-				},
-				[](SSACFG::BasicBlock::Jump const&) {},
-				[&](SSACFG::BasicBlock::ConditionalJump const& _conditionalJump) {
-					if (literalsFilter(m_cfg)(_conditionalJump.condition))
-						live.emplace(_conditionalJump.condition);
-				},
-				[](SSACFG::BasicBlock::Terminated const&) {}
-			};
-			std::visit(exitVisitor, block.exit);
+			live += blockExitValues(blockId);
 
 			for (auto const& op: block.operations | ranges::views::reverse)
 			{
@@ -163,12 +165,13 @@ void SSACFGLiveness::fillOperationsLiveOut()
 {
 	for (size_t blockIdValue = 0; blockIdValue < m_cfg.numBlocks(); ++blockIdValue)
 	{
-		auto const& operations = m_cfg.block(SSACFG::BlockId{blockIdValue}).operations;
+		SSACFG::BlockId const blockId{blockIdValue};
+		auto const& operations = m_cfg.block(blockId).operations;
 		auto& liveOuts = m_operationLiveOuts[blockIdValue];
 		liveOuts.resize(operations.size());
 		if (!operations.empty())
 		{
-			auto live = m_liveOuts[blockIdValue];
+			auto live = m_liveOuts[blockIdValue] + blockExitValues(blockId);
 			auto rit = liveOuts.rbegin();
 			for (auto const& op: operations | ranges::views::reverse)
 			{
diff --git a/libyul/backends/evm/SSACFGLiveness.h b/libyul/backends/evm/SSACFGLiveness.h
@@ -47,6 +47,7 @@ class SSACFGLiveness
 	void runDagDfs();
 	void runLoopTreeDfs(size_t _loopHeader);
 	void fillOperationsLiveOut();
+	std::set<SSACFG::ValueId> blockExitValues(SSACFG::BlockId const& _blockId) const;
 
 	SSACFG const& m_cfg;
 	ForwardSSACFGTopologicalSort m_topologicalSort;
diff --git a/libyul/backends/evm/SSACFGTopologicalSort.cpp b/libyul/backends/evm/SSACFGTopologicalSort.cpp
@@ -28,11 +28,7 @@ ForwardSSACFGTopologicalSort::ForwardSSACFGTopologicalSort(SSACFG const& _cfg):
 	yulAssert(m_cfg.entry.value == 0);
 	m_preOrder.reserve(m_cfg.numBlocks());
 	m_postOrder.reserve(m_cfg.numBlocks());
-	for (size_t id = 0; id < m_cfg.numBlocks(); ++id)
-	{
-		if (!m_explored[id])
-			dfs(id);
-	}
+	dfs(0);
 
 	for (auto const& [v1, v2]: m_potentialBackEdges)
 		if (ancestor(v2, v1))
diff --git a/libyul/backends/evm/SSAControlFlowGraph.cpp b/libyul/backends/evm/SSAControlFlowGraph.cpp
@@ -77,6 +77,25 @@ class SSACFGPrinter
 		);
 	}
 
+	static std::string escape(std::string_view const str)
+	{
+		using namespace std::literals;
+		static constexpr auto replacements = std::array{std::make_tuple('$', "_d_")};
+		std::stringstream ss;
+		for (auto const c: str)
+		{
+			auto const it = std::find_if(replacements.begin(), replacements.end(), [c](auto const& replacement)
+			{
+				return std::get<0>(replacement) == c;
+			});
+			if (it != replacements.end())
+				ss << std::get<1>(*it);
+			else
+				ss << c;
+		}
+		return ss.str();
+	}
+
 	std::string formatBlockHandle(SSACFG::BlockId const& _id) const
 	{
 		return fmt::format("Block{}_{}", m_functionIndex, _id.value);
@@ -157,7 +176,7 @@ class SSACFGPrinter
 					);
 				m_result << fmt::format(
 					"{}({})\\l\\\n",
-					label,
+					escape(label),
 					fmt::join(operation.inputs | ranges::views::transform(valueToString), ", ")
 				);
 			}
@@ -247,15 +266,15 @@ class SSACFGPrinter
 
 	void printFunction(Scope::Function const& _fun)
 	{
-		static auto constexpr returnsTransform = [](auto const& functionReturnValue) { return functionReturnValue.get().name.str(); };
+		static auto constexpr returnsTransform = [](auto const& functionReturnValue) { return escape(functionReturnValue.get().name.str()); };
 		static auto constexpr argsTransform = [](auto const& arg) { return fmt::format("v{}", std::get<1>(arg).value); };
-		m_result << "FunctionEntry_" << _fun.name.str() << "_" << m_cfg.entry.value << " [label=\"";
+		m_result << "FunctionEntry_" << escape(_fun.name.str()) << "_" << m_cfg.entry.value << " [label=\"";
 		if (!m_cfg.returns.empty())
-			m_result << fmt::format("function {0}:\n {1} := {0}({2})", _fun.name.str(), fmt::join(m_cfg.returns | ranges::views::transform(returnsTransform), ", "), fmt::join(m_cfg.arguments | ranges::views::transform(argsTransform), ", "));
+			m_result << fmt::format("function {0}:\n {1} := {0}({2})", escape(_fun.name.str()), fmt::join(m_cfg.returns | ranges::views::transform(returnsTransform), ", "), fmt::join(m_cfg.arguments | ranges::views::transform(argsTransform), ", "));
 		else
-			m_result << fmt::format("function {0}:\n {0}({1})", _fun.name.str(), fmt::join(m_cfg.arguments | ranges::views::transform(argsTransform), ", "));
+			m_result << fmt::format("function {0}:\n {0}({1})", escape(_fun.name.str()), fmt::join(m_cfg.arguments | ranges::views::transform(argsTransform), ", "));
 		m_result << "\"];\n";
-		m_result << "FunctionEntry_" << _fun.name.str() << "_" << m_cfg.entry.value << " -> Block" << m_functionIndex << "_" << m_cfg.entry.value << ";\n";
+		m_result << "FunctionEntry_" << escape(_fun.name.str()) << "_" << m_cfg.entry.value << " -> Block" << m_functionIndex << "_" << m_cfg.entry.value << ";\n";
 		printBlock(m_cfg.entry);
 	}
 
diff --git a/libyul/backends/evm/SSAControlFlowGraph.h b/libyul/backends/evm/SSAControlFlowGraph.h
@@ -76,7 +76,7 @@ class SSACFG
 		langutil::DebugData::ConstPtr debugData;
 		std::reference_wrapper<Scope::Function const> function;
 		std::reference_wrapper<FunctionCall const> call;
-		bool const canContinue = true;
+		bool canContinue;
 	};
 
 	struct Operation {
@@ -162,6 +162,10 @@ class SSACFG
 	};
 	struct UnreachableValue {};
 	using ValueInfo = std::variant<UnreachableValue, VariableValue, LiteralValue, PhiValue>;
+	bool isLiteralValue(ValueId const _var) const
+	{
+		return std::holds_alternative<LiteralValue>(valueInfo(_var));
+	}
 	ValueInfo& valueInfo(ValueId const _var)
 	{
 		return m_valueInfos.at(_var.value);
@@ -208,6 +212,14 @@ class SSACFG
 		return it->second;
 	}
 
+	size_t phiArgumentIndex(BlockId const _source, BlockId const _target) const
+	{
+		auto const& targetBlock = block(_target);
+		auto idx = util::findOffset(targetBlock.entries, _source);
+		yulAssert(idx, fmt::format("Target block {} not found as entry in one of the exits of the current block {}.", _target.value, _source.value));
+		return *idx;
+	}
+
 	std::string toDot(
 		bool _includeDiGraphDefinition=true,
 		std::optional<size_t> _functionIndex=std::nullopt,
