Improved branch handling in data flow analyzer.

libyul/optimiser/CommonSubexpressionEliminator.cpp

@@ -23,6 +23,7 @@
 
 #include <libyul/optimiser/SyntacticalEquality.h>
 #include <libyul/optimiser/CallGraphGenerator.h>
+#include <libyul/ControlFlowSideEffectsCollector.h>
 #include <libyul/optimiser/Semantics.h>
 #include <libyul/SideEffects.h>
 #include <libyul/Exceptions.h>
@@ -37,18 +38,13 @@ using namespace solidity::util;
 
 void CommonSubexpressionEliminator::run(OptimiserStepContext& _context, Block& _ast)
 {
-	CommonSubexpressionEliminator cse{
+	CommonSubexpressionEliminator{_context.dialect, _ast}(_ast);
-		_context.dialect,
-		SideEffectsPropagator::sideEffects(_context.dialect, CallGraphGenerator::callGraph(_ast))
-	};
-	cse(_ast);
 }
 
 CommonSubexpressionEliminator::CommonSubexpressionEliminator(
 	Dialect const& _dialect,
-	map<YulString, SideEffects> _functionSideEffects
+	Block const& _ast
-):
+): DataFlowAnalyzer(_dialect, _ast)
-	DataFlowAnalyzer(_dialect, std::move(_functionSideEffects))
 {
 }
 

libyul/optimiser/CommonSubexpressionEliminator.h

@@ -49,10 +49,7 @@ public:
 	void operator()(FunctionDefinition&) override;
 
 private:
-	CommonSubexpressionEliminator(
+	CommonSubexpressionEliminator(Dialect const& _dialect, Block const& _ast);
-		Dialect const& _dialect,
-		std::map<YulString, SideEffects> _functionSideEffects
-	);
 
 protected:
 	using ASTModifier::visit;

libyul/optimiser/DataFlowAnalyzer.cpp

@@ -25,6 +25,7 @@
 #include <libyul/optimiser/NameCollector.h>
 #include <libyul/optimiser/Semantics.h>
 #include <libyul/optimiser/KnowledgeBase.h>
+#include <libyul/ControlFlowSideEffectsCollector.h>
 #include <libyul/AST.h>
 #include <libyul/Dialect.h>
 #include <libyul/Exceptions.h>
@@ -42,14 +43,13 @@ using namespace solidity;
 using namespace solidity::util;
 using namespace solidity::yul;
 
-DataFlowAnalyzer::DataFlowAnalyzer(
+DataFlowAnalyzer::DataFlowAnalyzer(Dialect const& _dialect, Block const& _ast):
-	Dialect const& _dialect,
-	map<YulString, SideEffects> _functionSideEffects
-):
 	m_dialect(_dialect),
-	m_functionSideEffects(std::move(_functionSideEffects)),
 	m_knowledgeBase(_dialect, [this](YulString _var) { return variableValue(_var); })
 {
+	m_functionSideEffects = SideEffectsPropagator::sideEffects(_dialect, CallGraphGenerator::callGraph(_ast));
+	m_controlFlowSideEffects = ControlFlowSideEffectsCollector{_dialect, _ast}.functionSideEffectsNamed();
+
 	if (auto const* builtin = _dialect.memoryStoreFunction(YulString{}))
 		m_storeFunctionName[static_cast<unsigned>(StoreLoadLocation::Memory)] = builtin->name;
 	if (auto const* builtin = _dialect.memoryLoadFunction(YulString{}))
@@ -117,36 +117,51 @@ void DataFlowAnalyzer::operator()(VariableDeclaration& _varDecl)
 void DataFlowAnalyzer::operator()(If& _if)
 {
 	clearKnowledgeIfInvalidated(*_if.condition);
-	unordered_map<YulString, YulString> storage = m_state.storage;
+
-	unordered_map<YulString, YulString> memory = m_state.memory;
+	State preState = m_state;
 
 	ASTModifier::operator()(_if);
 
-	joinKnowledge(storage, memory);
+	if (hasFlowOutControlFlow(_if.body))
-
+	{
-	clearValues(assignedVariableNames(_if.body));
+		joinKnowledge(preState);
+		clearValues(assignedVariableNames(_if.body));
+	}
+	else
+		m_state = move(preState);
 }
 
 void DataFlowAnalyzer::operator()(Switch& _switch)
 {
 	clearKnowledgeIfInvalidated(*_switch.expression);
 	visit(*_switch.expression);
-	set<YulString> assignedVariables;
+
+	State preState = m_state;
+
+	optional<State> postState;
+	if (!hasDefaultCase(_switch))
+		postState = m_state;
+
+	std::set<YulString> assignedVariables;
 	for (auto& _case: _switch.cases)
 	{
-		unordered_map<YulString, YulString> storage = m_state.storage;
+		m_state = preState;
-		unordered_map<YulString, YulString> memory = m_state.memory;
 		(*this)(_case.body);
-		joinKnowledge(storage, memory);
 
-		set<YulString> variables = assignedVariableNames(_case.body);
+		if (hasFlowOutControlFlow(_case.body))
-		assignedVariables += variables;
+		{
-		// This is a little too destructive, we could retain the old values.
+			if (postState)
-		clearValues(variables);
+				joinKnowledge(*postState);
-		clearKnowledgeIfInvalidated(_case.body);
+			assignedVariables += assignedVariableNames(_case.body);
+
+			postState = move(m_state);
+		}
 	}
-	for (auto& _case: _switch.cases)
+	if (postState)
-		clearKnowledgeIfInvalidated(_case.body);
+		m_state = move(*postState);
+	else
+		// No outflowing case.
+		m_state = {};
 	clearValues(assignedVariables);
 }
 
@@ -362,30 +377,6 @@ void DataFlowAnalyzer::clearKnowledgeIfInvalidated(Expression const& _expr)
 		m_state.memory.clear();
 }
 
-void DataFlowAnalyzer::joinKnowledge(
-	unordered_map<YulString, YulString> const& _olderStorage,
-	unordered_map<YulString, YulString> const& _olderMemory
-)
-{
-	joinKnowledgeHelper(m_state.storage, _olderStorage);
-	joinKnowledgeHelper(m_state.memory, _olderMemory);
-}
-
-void DataFlowAnalyzer::joinKnowledgeHelper(
-	std::unordered_map<YulString, YulString>& _this,
-	std::unordered_map<YulString, YulString> const& _older
-)
-{
-	// We clear if the key does not exist in the older map or if the value is different.
-	// This also works for memory because _older is an "older version"
-	// of m_state.memory and thus any overlapping write would have cleared the keys
-	// that are not known to be different inside m_state.memory already.
-	cxx20::erase_if(_this, mapTuple([&_older](auto&& key, auto&& currentValue){
-		YulString const* oldValue = util::valueOrNullptr(_older, key);
-		return !oldValue || *oldValue != currentValue;
-	}));
-}
-
 bool DataFlowAnalyzer::inScope(YulString _variableName) const
 {
 	for (auto const& scope: m_variableScopes | ranges::views::reverse)
@@ -430,3 +421,32 @@ std::optional<YulString> DataFlowAnalyzer::isSimpleLoad(
 				return key->name;
 	return {};
 }
+
+bool DataFlowAnalyzer::hasFlowOutControlFlow(Block const& _block) const
+{
+	return
+		_block.statements.empty() ||
+		TerminationFinder{m_dialect, &m_controlFlowSideEffects}.
+			controlFlowKind(_block.statements.back()) == TerminationFinder::ControlFlow::FlowOut;
+}
+
+void DataFlowAnalyzer::joinKnowledge(State const& _olderState)
+{
+	joinKnowledgeHelper(m_state.storage, _olderState.storage);
+	joinKnowledgeHelper(m_state.memory, _olderState.memory);
+}
+
+void DataFlowAnalyzer::joinKnowledgeHelper(
+	std::unordered_map<YulString, YulString>& _this,
+	std::unordered_map<YulString, YulString> const& _older
+)
+{
+	// We clear if the key does not exist in the older map or if the value is different.
+	// This also works for memory because _older is an "older version"
+	// of m_state.memory and thus any overlapping write would have cleared the keys
+	// that are not known to be different inside m_state.memory already.
+	cxx20::erase_if(_this, mapTuple([&_older](auto&& key, auto&& currentValue){
+		YulString const* oldValue = valueOrNullptr(_older, key);
+		return !oldValue || *oldValue != currentValue;
+	}));
+}

libyul/optimiser/DataFlowAnalyzer.h

@@ -28,6 +28,7 @@
 #include <libyul/YulString.h>
 #include <libyul/AST.h> // Needed for m_zero below.
 #include <libyul/SideEffects.h>
+#include <libyul/ControlFlowSideEffects.h>
 
 #include <libsolutil/Numeric.h>
 #include <libsolutil/Common.h>
@@ -81,14 +82,10 @@ struct AssignedValue
 class DataFlowAnalyzer: public ASTModifier
 {
 public:
-	/// @param _functionSideEffects
+	/// Construct the data flow analyzer. The passed block should be the full AST
-	///            Side-effects of user-defined functions. Worst-case side-effects are assumed
+	/// because side effects of user defined functions are computed from it.
-	///            if this is not provided or the function is not found.
+	/// Otherwise, worst-case side effects are assumed.
-	///            The parameter is mostly used to determine movability of expressions.
+	DataFlowAnalyzer(Dialect const& _dialect, Block const& _ast);
-	explicit DataFlowAnalyzer(
-		Dialect const& _dialect,
-		std::map<YulString, SideEffects> _functionSideEffects = {}
-	);
 
 	using ASTModifier::operator();
 	void operator()(ExpressionStatement& _statement) override;
@@ -129,19 +126,6 @@ protected:
 	/// Clears knowledge about storage or memory if they may be modified inside the expression.
 	void clearKnowledgeIfInvalidated(Expression const& _expression);
 
-	/// Joins knowledge about storage and memory with an older point in the control-flow.
-	/// This only works if the current state is a direct successor of the older point,
-	/// i.e. `_otherStorage` and `_otherMemory` cannot have additional changes.
-	void joinKnowledge(
-		std::unordered_map<YulString, YulString> const& _olderStorage,
-		std::unordered_map<YulString, YulString> const& _olderMemory
-	);
-
-	static void joinKnowledgeHelper(
-		std::unordered_map<YulString, YulString>& _thisData,
-		std::unordered_map<YulString, YulString> const& _olderData
-	);
-
 	/// Returns true iff the variable is in scope.
 	bool inScope(YulString _variableName) const;
 
@@ -168,10 +152,15 @@ protected:
 		Expression const& _expression
 	) const;
 
+	/// @returns true if the block is empty or the last statement has a
+	/// "flow out" control flow.
+	bool hasFlowOutControlFlow(Block const& _block) const;
+
 	Dialect const& m_dialect;
 	/// Side-effects of user-defined functions. Worst-case side-effects are assumed
 	/// if this is not provided or the function is not found.
 	std::map<YulString, SideEffects> m_functionSideEffects;
+	std::map<YulString, ControlFlowSideEffects> m_controlFlowSideEffects;
 
 private:
 	struct State
@@ -184,6 +173,18 @@ private:
 		std::unordered_map<YulString, YulString> storage;
 		std::unordered_map<YulString, YulString> memory;
 	};
+
+	/// Joins knowledge about storage and memory with an older point in the control-flow.
+	/// This only works if the current state is a direct successor of the older point,
+	/// i.e. `_otherStorage` and `_otherMemory` cannot have additional changes.
+	void joinKnowledge(State const& _olderState);
+
+	static void joinKnowledgeHelper(
+		std::unordered_map<YulString, YulString>& _thisData,
+		std::unordered_map<YulString, YulString> const& _olderData
+	);
+
+
 	State m_state;
 
 protected:

libyul/optimiser/EqualStoreEliminator.cpp

@@ -25,6 +25,7 @@
 #include <libyul/optimiser/CallGraphGenerator.h>
 #include <libyul/optimiser/OptimizerUtilities.h>
 #include <libyul/optimiser/Semantics.h>
+#include <libyul/ControlFlowSideEffectsCollector.h>
 #include <libyul/AST.h>
 #include <libyul/Utilities.h>
 
@@ -36,10 +37,7 @@ using namespace solidity::yul;
 
 void EqualStoreEliminator::run(OptimiserStepContext const& _context, Block& _ast)
 {
-	EqualStoreEliminator eliminator{
+	EqualStoreEliminator eliminator{_context.dialect, _ast};
-		_context.dialect,
-		SideEffectsPropagator::sideEffects(_context.dialect, CallGraphGenerator::callGraph(_ast))
-	};
 	eliminator(_ast);
 
 	StatementRemover remover{eliminator.m_pendingRemovals};

libyul/optimiser/EqualStoreEliminator.h

@@ -43,11 +43,8 @@ public:
 	static void run(OptimiserStepContext const&, Block& _ast);
 
 private:
-	EqualStoreEliminator(
+	EqualStoreEliminator(Dialect const& _dialect, Block const& _ast):
-		Dialect const& _dialect,
+		DataFlowAnalyzer(_dialect, _ast)
-		std::map<YulString, SideEffects> _functionSideEffects
-	):
-		DataFlowAnalyzer(_dialect, std::move(_functionSideEffects))
 	{}
 
 protected:

libyul/optimiser/ExpressionSimplifier.cpp

@@ -23,6 +23,8 @@
 
 #include <libyul/optimiser/SimplificationRules.h>
 #include <libyul/optimiser/OptimiserStep.h>
+#include <libyul/optimiser/Semantics.h>
+#include <libyul/ControlFlowSideEffectsCollector.h>
 #include <libyul/AST.h>
 
 using namespace std;
@@ -31,7 +33,7 @@ using namespace solidity::yul;
 
 void ExpressionSimplifier::run(OptimiserStepContext& _context, Block& _ast)
 {
-	ExpressionSimplifier{_context.dialect}(_ast);
+	ExpressionSimplifier{_context.dialect, _ast}(_ast);
 }
 
 void ExpressionSimplifier::visit(Expression& _expression)

libyul/optimiser/ExpressionSimplifier.h

@@ -51,7 +51,9 @@ public:
 	void visit(Expression& _expression) override;
 
 private:
-	explicit ExpressionSimplifier(Dialect const& _dialect): DataFlowAnalyzer(_dialect) {}
+	ExpressionSimplifier(Dialect const& _dialect, Block const& _ast):
+		DataFlowAnalyzer(_dialect, _ast)
+	{}
 };
 
 }

libyul/optimiser/LoadResolver.cpp

@@ -26,6 +26,7 @@
 #include <libyul/backends/evm/EVMMetrics.h>
 #include <libyul/optimiser/Semantics.h>
 #include <libyul/optimiser/CallGraphGenerator.h>
+#include <libyul/ControlFlowSideEffectsCollector.h>
 #include <libyul/SideEffects.h>
 #include <libyul/AST.h>
 #include <libyul/Utilities.h>
@@ -46,7 +47,7 @@ void LoadResolver::run(OptimiserStepContext& _context, Block& _ast)
 	bool containsMSize = MSizeFinder::containsMSize(_context.dialect, _ast);
 	LoadResolver{
 		_context.dialect,
-		SideEffectsPropagator::sideEffects(_context.dialect, CallGraphGenerator::callGraph(_ast)),
+		_ast,
 		containsMSize,
 		_context.expectedExecutionsPerDeployment
 	}(_ast);

libyul/optimiser/LoadResolver.h

@@ -49,11 +49,11 @@ public:
 private:
 	LoadResolver(
 		Dialect const& _dialect,
-		std::map<YulString, SideEffects> _functionSideEffects,
+		Block const& _ast,
 		bool _containsMSize,
 		std::optional<size_t> _expectedExecutionsPerDeployment
 	):
-		DataFlowAnalyzer(_dialect, std::move(_functionSideEffects)),
+		DataFlowAnalyzer(_dialect, _ast),
 		m_containsMSize(_containsMSize),
 		m_expectedExecutionsPerDeployment(std::move(_expectedExecutionsPerDeployment))
 	{}

libyul/optimiser/Rematerialiser.cpp

@@ -43,7 +43,7 @@ Rematerialiser::Rematerialiser(
 	set<YulString> _varsToAlwaysRematerialize,
 	bool _onlySelectedVariables
 ):
-	DataFlowAnalyzer(_dialect),
+	DataFlowAnalyzer(_dialect, _ast),
 	m_referenceCounts(ReferencesCounter::countReferences(_ast)),
 	m_varsToAlwaysRematerialize(std::move(_varsToAlwaysRematerialize)),
 	m_onlySelectedVariables(_onlySelectedVariables)
@@ -87,6 +87,11 @@ void Rematerialiser::visit(Expression& _e)
 	DataFlowAnalyzer::visit(_e);
 }
 
+void LiteralRematerialiser::run(OptimiserStepContext& _context, Block& _ast)
+{
+	LiteralRematerialiser{_context.dialect, _ast}(_ast);
+}
+
 void LiteralRematerialiser::visit(Expression& _e)
 {
 	if (holds_alternative<Identifier>(_e))

libyul/optimiser/Rematerialiser.h

@@ -23,6 +23,8 @@
 
 #include <libyul/optimiser/DataFlowAnalyzer.h>
 #include <libyul/optimiser/OptimiserStep.h>
+#include <libyul/optimiser/Semantics.h>
+#include <libyul/ControlFlowSideEffectsCollector.h>
 
 namespace solidity::yul
 {
@@ -85,17 +87,14 @@ class LiteralRematerialiser: public DataFlowAnalyzer
 {
 public:
 	static constexpr char const* name{"LiteralRematerialiser"};
-	static void run(
+	static void run(OptimiserStepContext& _context, Block& _ast);
-		OptimiserStepContext& _context,
-		Block& _ast
-	) { LiteralRematerialiser{_context.dialect}(_ast); }
 
 	using ASTModifier::visit;
 	void visit(Expression& _e) override;
 
 private:
-	LiteralRematerialiser(Dialect const& _dialect):
+	LiteralRematerialiser(Dialect const& _dialect, Block const& _ast):
-		DataFlowAnalyzer(_dialect)
+		DataFlowAnalyzer(_dialect, _ast)
 	{}
 };
 

libyul/optimiser/StackCompressor.cpp

@@ -56,7 +56,9 @@ namespace
 class RematCandidateSelector: public DataFlowAnalyzer
 {
 public:
-	explicit RematCandidateSelector(Dialect const& _dialect): DataFlowAnalyzer(_dialect) {}
+	RematCandidateSelector(Dialect const& _dialect, Block const& _ast):
+		DataFlowAnalyzer(_dialect, _ast)
+	{}
 
 	/// @returns a map from function name to rematerialisation costs to a vector of variables to rematerialise
 	/// and variables that occur in their expression.
@@ -165,7 +167,7 @@ void eliminateVariables(
 	bool _allowMSizeOptimization
 )
 {
-	RematCandidateSelector selector{_dialect};
+	RematCandidateSelector selector{_dialect, _ast};
 	selector(_ast);
 	map<YulString, map<size_t, vector<YulString>>> candidates = selector.candidates();
 
@@ -192,7 +194,7 @@ void eliminateVariablesOptimizedCodegen(
 	if (std::all_of(_unreachables.begin(), _unreachables.end(), [](auto const& _item) { return _item.second.empty(); }))
 		return;
 
-	RematCandidateSelector selector{_dialect};
+	RematCandidateSelector selector{_dialect, _ast};
 	selector(_ast);
 
 	map<YulString, size_t> candidates;

test/libyul/yulOptimizerTests/fullSuite/revert_in_switch.yul

@@ -0,0 +1,21 @@
+{
+    let x := 0
+    switch calldataload(0)
+    case 0 {
+        x := calldataload(99)
+    }
+    case 1 {
+        if 0 { revert(0, 0) }
+    }
+    sstore(0, x)
+}
+// ----
+// step: fullSuite
+//
+// {
+//     {
+//         let x := 0
+//         if iszero(calldataload(x)) { x := calldataload(99) }
+//         sstore(0, x)
+//     }
+// }

test/libyul/yulOptimizerTests/loadResolver/mload_with_revert.yul

@@ -0,0 +1,29 @@
+{
+    let b := mload(2)
+    mstore(0, b)
+    if calldataload(1) {
+        mstore(2, 7)
+        mstore(0, 7)
+        return(0, 0)
+    }
+    sstore(0, mload(0))
+}
+// ----
+// step: loadResolver
+//
+// {
+//     {
+//         let _1 := 2
+//         let b := mload(_1)
+//         let _2 := 0
+//         mstore(_2, b)
+//         if calldataload(1)
+//         {
+//             let _5 := 7
+//             mstore(_1, _5)
+//             mstore(_2, _5)
+//             return(_2, _2)
+//         }
+//         sstore(_2, b)
+//     }
+// }

test/libyul/yulOptimizerTests/rematerialiser/branches_switch.yul

@@ -16,7 +16,7 @@
 //     case 1 { b := 1 }
 //     default {
 //         let x := 1
-//         let y := b
+//         let y := 2
 //         b := 1
 //     }
 //     pop(add(1, b))