More logic about control flow with continue and about finalize. Remove BlockScope.

Author: chriseth

libyul/optimiser/RedundantAssignEliminator.cpp

@@ -97,31 +97,30 @@ void RedundantAssignEliminator::operator()(FunctionDefinition const& _functionDe
 {
 	std::set<YulString> outerDeclaredVariables;
 	TrackedAssignments outerAssignments;
+	ForLoopInfo forLoopInfo;
 	swap(m_declaredVariables, outerDeclaredVariables);
 	swap(m_assignments, outerAssignments);
+	swap(m_forLoopInfo, forLoopInfo);
 
 	(*this)(_functionDefinition.body);
 
 	for (auto const& param: _functionDefinition.parameters)
-	{
-		changeUndecidedTo(param.name, State::Unused);
-		finalize(param.name);
-	}
+		finalize(param.name, State::Unused);
 	for (auto const& retParam: _functionDefinition.returnVariables)
-	{
-		changeUndecidedTo(retParam.name, State::Used);
-		finalize(retParam.name);
-	}
+		finalize(retParam.name, State::Used);
 
 	swap(m_declaredVariables, outerDeclaredVariables);
 	swap(m_assignments, outerAssignments);
+	swap(m_forLoopInfo, forLoopInfo);
 }
 
 void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 {
-	// This will set all variables that are declared in this
-	// block to "unused" when it is destroyed.
-	BlockScope scope(*this);
+	ForLoopInfo outerForLoopInfo;
+	swap(outerForLoopInfo, m_forLoopInfo);
+
+	set<YulString> outerDeclaredVariables;
+	swap(m_declaredVariables, outerDeclaredVariables);
 
 	// We need to visit the statements directly because of the
 	// scoping rules.
@@ -133,9 +132,6 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 
 	visit(*_forLoop.condition);
 
-	ForLoopInfo outerForLoopInfo;
-	swap(outerForLoopInfo, m_forLoopInfo);
-
 	TrackedAssignments zeroRuns{m_assignments};
 
 	(*this)(_forLoop.body);
@@ -150,6 +146,7 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 	(*this)(_forLoop.body);
 
 	merge(m_assignments, move(m_forLoopInfo.pendingContinueStmts));
+	m_forLoopInfo.pendingContinueStmts = {};
 	(*this)(_forLoop.post);
 
 	visit(*_forLoop.condition);
@@ -159,27 +156,37 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 	merge(m_assignments, move(zeroRuns));
 	merge(m_assignments, move(m_forLoopInfo.pendingBreakStmts));
 
-	// Oestore potential outer for-loop states.
+	for (auto const& var: m_declaredVariables)
+		finalize(var, State::Unused);
+	swap(m_declaredVariables, outerDeclaredVariables);
+
+	// Restore potential outer for-loop states.
 	swap(m_forLoopInfo, outerForLoopInfo);
 }
 
 void RedundantAssignEliminator::operator()(Break const&)
 {
-	m_forLoopInfo.pendingBreakStmts.push_back(m_assignments);
+	m_forLoopInfo.pendingBreakStmts.emplace_back(move(m_assignments));
+	m_assignments.clear();
 }
 
 void RedundantAssignEliminator::operator()(Continue const&)
 {
-	m_forLoopInfo.pendingContinueStmts.push_back(m_assignments);
+	m_forLoopInfo.pendingContinueStmts.emplace_back(move(m_assignments));
+	m_assignments.clear();
 }
 
 void RedundantAssignEliminator::operator()(Block const& _block)
 {
-	// This will set all variables that are declared in this
-	// block to "unused" when it is destroyed.
-	BlockScope scope(*this);
+	set<YulString> outerDeclaredVariables;
+	swap(m_declaredVariables, outerDeclaredVariables);
 
 	ASTWalker::operator()(_block);
+
+	for (auto const& var: m_declaredVariables)
+		finalize(var, State::Unused);
+
+	swap(m_declaredVariables, outerDeclaredVariables);
 }
 
 void RedundantAssignEliminator::run(Dialect const& _dialect, Block& _ast)
@@ -239,21 +246,38 @@ void RedundantAssignEliminator::merge(TrackedAssignments& _target, vector<Tracke
 void RedundantAssignEliminator::changeUndecidedTo(YulString _variable, RedundantAssignEliminator::State _newState)
 {
 	for (auto& assignment: m_assignments[_variable])
-		if (assignment.second == State{State::Undecided})
+		if (assignment.second == State::Undecided)
 			assignment.second = _newState;
 }
 
-void RedundantAssignEliminator::finalize(YulString _variable)
+void RedundantAssignEliminator::finalize(YulString _variable, RedundantAssignEliminator::State _finalState)
 {
-	for (auto& assignment: m_assignments[_variable])
+	finalize(m_assignments, _variable, _finalState);
+	for (auto& assignments: m_forLoopInfo.pendingBreakStmts)
+		finalize(assignments, _variable, _finalState);
+	for (auto& assignments: m_forLoopInfo.pendingContinueStmts)
+		finalize(assignments, _variable, _finalState);
+}
+
+void RedundantAssignEliminator::finalize(
+	TrackedAssignments& _assignments,
+	YulString _variable,
+	RedundantAssignEliminator::State _finalState
+)
+{
+	for (auto const& assignment: _assignments[_variable])
 	{
-		assertThrow(assignment.second != State::Undecided, OptimizerException, "");
-		if (assignment.second == State{State::Unused} && MovableChecker{*m_dialect, *assignment.first->value}.movable())
+		State state = assignment.second;
+		if (state == State::Undecided)
+			state = _finalState;
+
+		assertThrow(state != State::Undecided, OptimizerException, "");
+		if (state == State::Unused && MovableChecker{*m_dialect, *assignment.first->value}.movable())
 			// TODO the only point where we actually need this
 			// to be a set is for the for loop
 			m_pendingRemovals.insert(assignment.first);
 	}
-	m_assignments.erase(_variable);
+	_assignments.erase(_variable);
 }
 
 void AssignmentRemover::operator()(Block& _block)

libyul/optimiser/RedundantAssignEliminator.h

@@ -137,33 +137,6 @@ class RedundantAssignEliminator: public ASTWalker
 		Value m_value = Undecided;
 	};
 
-	/**
-	 * Takes care about storing the list of declared variables and
-	 * sets them to "unused" when it is destroyed.
-	 */
-	class BlockScope
-	{
-	public:
-		explicit BlockScope(RedundantAssignEliminator& _rae): m_rae(_rae)
-		{
-			swap(m_rae.m_declaredVariables, m_outerDeclaredVariables);
-		}
-		~BlockScope()
-		{
-			// This should actually store all declared variables
-			// into a different mapping
-			for (auto const& var: m_rae.m_declaredVariables)
-				m_rae.changeUndecidedTo(var, State::Unused);
-			for (auto const& var: m_rae.m_declaredVariables)
-				m_rae.finalize(var);
-			swap(m_rae.m_declaredVariables, m_outerDeclaredVariables);
-		}
-
-	private:
-		RedundantAssignEliminator& m_rae;
-		std::set<YulString> m_outerDeclaredVariables;
-	};
-
 	// TODO check that this does not cause nondeterminism!
 	// This could also be a pseudo-map from state to assignment.
 	using TrackedAssignments = std::map<YulString, std::map<Assignment const*, State>>;
@@ -174,7 +147,12 @@ class RedundantAssignEliminator: public ASTWalker
 	static void merge(TrackedAssignments& _target, TrackedAssignments&& _source);
 	static void merge(TrackedAssignments& _target, std::vector<TrackedAssignments>&& _source);
 	void changeUndecidedTo(YulString _variable, State _newState);
-	void finalize(YulString _variable);
+	/// Called when a variables goes out of scope. Sets the state of all still undecided
+	/// assignments to the final state. In this case, this also applies to pending
+	/// break and continue TrackedAssignments.
+	void finalize(YulString _variable, State _finalState);
+	/// Helper function for the above.
+	void finalize(TrackedAssignments& _assignments, YulString _variable, State _finalState);
 
 	Dialect const* m_dialect;
 	std::set<YulString> m_declaredVariables;

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_break.yul

@@ -1,6 +1,7 @@
 {
-	// Cannot be removed, because we might run the loop only once
-	let x := 1
+	let x
+	// Cannot be removed, because we might skip the loop
+	x := 1
 	for { } calldataload(0) { }
 	{
 		if callvalue() {
@@ -14,7 +15,8 @@
 // ----
 // redundantAssignEliminator
 // {
-//     let x := 1
+//     let x
+//     x := 1
 //     for {
 //     }
 //     calldataload(0)

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue.yul

@@ -1,10 +1,13 @@
 {
-	// Cannot be removed, because we might run the loop only once
-	let x := 1
+	let x
+	// Can be removed, because x is reassigned after the loop
+	x := 1
 	for { } calldataload(0) { }
 	{
 		x := 2 // Will not be removed as if-condition can be false.
 		if callvalue() {
+			// This can be removed because x is overwritten both after the
+			// loop at at the start of the next iteration.
 			x := 3
 			continue
 		}
@@ -15,7 +18,7 @@
 // ----
 // redundantAssignEliminator
 // {
-//     let x := 1
+//     let x
 //     for {
 //     }
 //     calldataload(0)
@@ -25,7 +28,6 @@
 //         x := 2
 //         if callvalue()
 //         {
-//             x := 3
 //             continue
 //         }
 //         mstore(x, 2)

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_2.yul

@@ -1,6 +1,7 @@
 {
-	// Cannot be removed, because we might run the loop only once
-	let x := 1
+	let x
+	// Cannot be removed, because we might skip the loop
+	x := 1
 	for { } calldataload(0) { }
 	{
 		if callvalue() {
@@ -14,7 +15,8 @@
 // ----
 // redundantAssignEliminator
 // {
-//     let x := 1
+//     let x
+//     x := 1
 //     for {
 //     }
 //     calldataload(0)

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_3.yul

@@ -1,6 +1,7 @@
 {
-	// Cannot be removed, because we might run the loop only once
-	let x := 1
+	let x
+	// Can be removed, because x is not used after the loop.
+	x := 1
 	for { } calldataload(0) { mstore(x, 0x42) }
 	{
 		if callvalue() {
@@ -13,7 +14,7 @@
 // ----
 // redundantAssignEliminator
 // {
-//     let x := 1
+//     let x
 //     for {
 //     }
 //     calldataload(0)

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_decl_inside_break_continue.yul

@@ -0,0 +1,44 @@
+{
+	let x := 1
+	for { } calldataload(0) { }
+	{
+        // This will go out of scope at the end of the block,
+        // but the continue/break statements still refer to it.
+        {
+            let y := 9
+            if callvalue() {
+                y := 2 // will be removed
+                break
+            }
+            if eq(callvalue(), 3) {
+                y := 12 // will be removed
+                continue
+            }
+        }
+	}
+	mstore(x, 0x42)
+}
+// ----
+// redundantAssignEliminator
+// {
+//     let x := 1
+//     for {
+//     }
+//     calldataload(0)
+//     {
+//     }
+//     {
+//         {
+//             let y := 9
+//             if callvalue()
+//             {
+//                 break
+//             }
+//             if eq(callvalue(), 3)
+//             {
+//                 continue
+//             }
+//         }
+//     }
+//     mstore(x, 0x42)
+// }