Take control flow side effects into account.

ethereum · Mar 10, 2022 · 5fc9b74 · 5fc9b74
1 parent c09fe0b
commit 5fc9b74
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 13 deletions.
diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp
@@ -24,6 +24,7 @@
 
 #include <libyul/optimiser/Semantics.h>
 #include <libyul/optimiser/OptimizerUtilities.h>
+#include <libyul/ControlFlowSideEffectsCollector.h>
 #include <libyul/AST.h>
 
 #include <libsolutil/CommonData.h>
@@ -36,7 +37,10 @@ using namespace solidity::yul;
 
 void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast)
 {
-	UnusedAssignEliminator rae{_context.dialect};
+	UnusedAssignEliminator rae{
+		_context.dialect,
+		ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed()
+	};
 	rae(_ast);
 
 	StatementRemover remover{rae.m_pendingRemovals};

diff --git a/libyul/optimiser/UnusedAssignEliminator.h b/libyul/optimiser/UnusedAssignEliminator.h
@@ -113,7 +113,10 @@ class UnusedAssignEliminator: public UnusedStoreBase
 	static constexpr char const* name{"UnusedAssignEliminator"};
 	static void run(OptimiserStepContext&, Block& _ast);
 
-	explicit UnusedAssignEliminator(Dialect const& _dialect): UnusedStoreBase(_dialect) {}
+	explicit UnusedAssignEliminator(
+		Dialect const& _dialect,
+		std::map<YulString, ControlFlowSideEffects> _controlFlowSideEffects
+	): UnusedStoreBase(_dialect, std::move(_controlFlowSideEffects)) {}
 
 	void operator()(Identifier const& _identifier) override;
 	void operator()(VariableDeclaration const& _variableDeclaration) override;

diff --git a/libyul/optimiser/UnusedStoreBase.cpp b/libyul/optimiser/UnusedStoreBase.cpp
@@ -40,7 +40,10 @@ void UnusedStoreBase::operator()(If const& _if)
 	TrackedStores skipBranch{m_stores};
 	(*this)(_if.body);
 
-	merge(m_stores, move(skipBranch));
+	if (continuesExecution(_if.body))
+		merge(m_stores, move(skipBranch));
+	else
+		m_stores = move(skipBranch);
 }
 
 void UnusedStoreBase::operator()(Switch const& _switch)
@@ -50,22 +53,30 @@ void UnusedStoreBase::operator()(Switch const& _switch)
 	TrackedStores const preState{m_stores};
 
 	bool hasDefault = false;
-	vector<TrackedStores> branches;
+	vector<TrackedStores> branchesToJoin;
 	for (auto const& c: _switch.cases)
 	{
 		if (!c.value)
 			hasDefault = true;
 		(*this)(c.body);
-		branches.emplace_back(move(m_stores));
+		if (continuesExecution(c.body))
+			branchesToJoin.emplace_back(move(m_stores));
 		m_stores = preState;
 	}
 
-	if (hasDefault)
+	if (!hasDefault)
+		branchesToJoin.emplace_back(preState);
+
+	if (branchesToJoin.empty())
+		// This happens if all branches terminate,
+		// it does not really matter what we do here.
+		m_stores = preState;
+	else
 	{
-		m_stores = move(branches.back());
-		branches.pop_back();
+		m_stores = move(branchesToJoin.back());
+		branchesToJoin.pop_back();
 	}
-	for (auto& branch: branches)
+	for (auto& branch: branchesToJoin)
 		merge(m_stores, move(branch));
 }
 
@@ -157,3 +168,11 @@ void UnusedStoreBase::merge(TrackedStores& _target, vector<TrackedStores>&& _sou
 		merge(_target, move(ts));
 	_source.clear();
 }
+
+bool UnusedStoreBase::continuesExecution(Block const& _block)
+{
+	return
+		_block.statements.empty() ||
+		TerminationFinder(m_dialect, &m_controlFlowSideEffects).controlFlowKind(_block.statements.back()) ==
+		TerminationFinder::ControlFlow::FlowOut;
+}
diff --git a/libyul/optimiser/UnusedStoreBase.h b/libyul/optimiser/UnusedStoreBase.h
@@ -22,6 +22,7 @@
 #pragma once
 
 #include <libyul/optimiser/ASTWalker.h>
+#include <libyul/ControlFlowSideEffects.h>
 #include <libyul/AST.h>
 
 #include <range/v3/action/remove_if.hpp>
@@ -46,7 +47,10 @@ struct Dialect;
 class UnusedStoreBase: public ASTWalker
 {
 public:
-	explicit UnusedStoreBase(Dialect const& _dialect): m_dialect(_dialect) {}
+	UnusedStoreBase(
+		Dialect const& _dialect,
+		std::map<YulString, ControlFlowSideEffects> _controlFlowSideEffects
+	): m_dialect(_dialect), m_controlFlowSideEffects(std::move(_controlFlowSideEffects)) {}
 
 	using ASTWalker::operator();
 	void operator()(If const& _if) override;
@@ -89,7 +93,10 @@ class UnusedStoreBase: public ASTWalker
 	static void merge(TrackedStores& _target, TrackedStores&& _source);
 	static void merge(TrackedStores& _target, std::vector<TrackedStores>&& _source);
 
+	bool continuesExecution(Block const& _block);
+
 	Dialect const& m_dialect;
+	std::map<YulString, ControlFlowSideEffects> m_controlFlowSideEffects;
 	std::set<Statement const*> m_pendingRemovals;
 	TrackedStores m_stores;
 

diff --git a/libyul/optimiser/UnusedStoreEliminator.h b/libyul/optimiser/UnusedStoreEliminator.h
@@ -62,10 +62,9 @@ class UnusedStoreEliminator: public UnusedStoreBase
 		std::map<YulString, AssignedValue> const& _ssaValues,
 		bool _ignoreMemory
 	):
-		UnusedStoreBase(_dialect),
+		UnusedStoreBase(_dialect, move(_controlFlowSideEffects)),
 		m_ignoreMemory(_ignoreMemory),
 		m_functionSideEffects(_functionSideEffects),
-		m_controlFlowSideEffects(_controlFlowSideEffects),
 		m_ssaValues(_ssaValues)
 	{}
 
@@ -110,7 +109,6 @@ class UnusedStoreEliminator: public UnusedStoreBase
 
 	bool const m_ignoreMemory;
 	std::map<YulString, SideEffects> const& m_functionSideEffects;
-	std::map<YulString, ControlFlowSideEffects> m_controlFlowSideEffects;
 	std::map<YulString, AssignedValue> const& m_ssaValues;
 
 	std::map<Statement const*, Operation> m_storeOperations;