Merge pull request #6315 from ethereum/yul-for-continue-tests

[Yul] RedundantAssignEliminator adoptions for continue/break AST nodes

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)
-	{
+		finalize(param.name, State::Unused);
-		changeUndecidedTo(param.name, State::Unused);
-		finalize(param.name);
-	}
 	for (auto const& retParam: _functionDefinition.returnVariables)
-	{
+		finalize(retParam.name, State::Used);
-		changeUndecidedTo(retParam.name, State::Used);
-		finalize(retParam.name);
-	}
 
 	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
+	ForLoopInfo outerForLoopInfo;
-	// block to "unused" when it is destroyed.
+	swap(outerForLoopInfo, m_forLoopInfo);
-	BlockScope scope(*this);
+
+	set<YulString> outerDeclaredVariables;
+	swap(m_declaredVariables, outerDeclaredVariables);
 
 	// We need to visit the statements directly because of the
 	// scoping rules.
@@ -136,6 +135,8 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 	TrackedAssignments zeroRuns{m_assignments};
 
 	(*this)(_forLoop.body);
+	merge(m_assignments, move(m_forLoopInfo.pendingContinueStmts));
+	m_forLoopInfo.pendingContinueStmts = {};
 	(*this)(_forLoop.post);
 
 	visit(*_forLoop.condition);
@@ -143,6 +144,9 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 	TrackedAssignments oneRun{m_assignments};
 
 	(*this)(_forLoop.body);
+
+	merge(m_assignments, move(m_forLoopInfo.pendingContinueStmts));
+	m_forLoopInfo.pendingContinueStmts.clear();
 	(*this)(_forLoop.post);
 
 	visit(*_forLoop.condition);
@@ -150,25 +154,40 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 	// Order does not matter because "max" is commutative and associative.
 	merge(m_assignments, move(oneRun));
 	merge(m_assignments, move(zeroRuns));
+	merge(m_assignments, move(m_forLoopInfo.pendingBreakStmts));
+	m_forLoopInfo.pendingBreakStmts.clear();
+
+	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&)
 {
-	yulAssert(false, "Not implemented yet.");
+	m_forLoopInfo.pendingBreakStmts.emplace_back(move(m_assignments));
+	m_assignments.clear();
 }
 
 void RedundantAssignEliminator::operator()(Continue const&)
 {
-	yulAssert(false, "Not implemented yet.");
+	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
+	set<YulString> outerDeclaredVariables;
-	// block to "unused" when it is destroyed.
+	swap(m_declaredVariables, outerDeclaredVariables);
-	BlockScope scope(*this);
 
 	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)
@@ -218,24 +237,45 @@ void RedundantAssignEliminator::merge(TrackedAssignments& _target, TrackedAssign
 	});
 }
 
+void RedundantAssignEliminator::merge(TrackedAssignments& _target, vector<TrackedAssignments>&& _source)
+{
+	for (TrackedAssignments& ts: _source)
+		merge(_target, move(ts));
+	_source.clear();
+}
+
 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, "");
+		State const state = assignment.second == State::Undecided ? _finalState : assignment.second;
-		if (assignment.second == State{State::Unused} && MovableChecker{*m_dialect, *assignment.first->value}.movable())
+
+		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

@@ -25,6 +25,7 @@
 #include <libyul/optimiser/ASTWalker.h>
 
 #include <map>
+#include <vector>
 
 namespace yul
 {
@@ -136,33 +137,6 @@ private:
 		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>>;
@@ -171,13 +145,29 @@ private:
 	/// above.
 	/// Will destroy @a _source.
 	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 variable 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;
 	std::set<Assignment const*> m_pendingRemovals;
 	TrackedAssignments m_assignments;
+
+	/// Working data for traversing for-loops.
+	struct ForLoopInfo
+	{
+		/// Tracked assignment states for each break statement.
+		std::vector<TrackedAssignments> pendingBreakStmts;
+		/// Tracked assignment states for each continue statement.
+		std::vector<TrackedAssignments> pendingContinueStmts;
+	};
+	ForLoopInfo m_forLoopInfo;
 };
 
 class AssignmentRemover: public ASTModifier

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_break.yul

@@ -0,0 +1,34 @@
+{
+    let x
+    // Cannot be removed, because we might skip the loop
+    x := 1
+    for { } calldataload(0) { }
+    {
+        if callvalue() {
+            x := 2 // is preserved because of break stmt below.
+            break
+        }
+        x := 3
+    }
+    mstore(x, 0x42)
+}
+// ----
+// redundantAssignEliminator
+// {
+//     let x
+//     x := 1
+//     for {
+//     }
+//     calldataload(0)
+//     {
+//     }
+//     {
+//         if callvalue()
+//         {
+//             x := 2
+//             break
+//         }
+//         x := 3
+//     }
+//     mstore(x, 0x42)
+// }

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue.yul

@@ -0,0 +1,35 @@
+{
+    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
+        }
+        mstore(x, 2)
+    }
+    x := 3
+}
+// ----
+// redundantAssignEliminator
+// {
+//     let x
+//     for {
+//     }
+//     calldataload(0)
+//     {
+//     }
+//     {
+//         x := 2
+//         if callvalue()
+//         {
+//             continue
+//         }
+//         mstore(x, 2)
+//     }
+// }

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_2.yul

@@ -0,0 +1,34 @@
+{
+    let x
+    // Cannot be removed, because we might skip the loop
+    x := 1
+    for { } calldataload(0) { }
+    {
+        if callvalue() {
+            x := 2 // is preserved because of continue stmt below.
+            continue
+        }
+        x := 3
+    }
+    mstore(x, 0x42)
+}
+// ----
+// redundantAssignEliminator
+// {
+//     let x
+//     x := 1
+//     for {
+//     }
+//     calldataload(0)
+//     {
+//     }
+//     {
+//         if callvalue()
+//         {
+//             x := 2
+//             continue
+//         }
+//         x := 3
+//     }
+//     mstore(x, 0x42)
+// }

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_3.yul

@@ -0,0 +1,32 @@
+{
+    let x
+    // Can be removed, because x is not used after the loop.
+    x := 1
+    for { } calldataload(0) { mstore(x, 0x42) }
+    {
+        if callvalue() {
+            x := 2 // is preserved because of continue stmt below.
+            continue
+        }
+        x := 3
+    }
+}
+// ----
+// redundantAssignEliminator
+// {
+//     let x
+//     for {
+//     }
+//     calldataload(0)
+//     {
+//         mstore(x, 0x42)
+//     }
+//     {
+//         if callvalue()
+//         {
+//             x := 2
+//             continue
+//         }
+//         x := 3
+//     }
+// }

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)
+// }

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_multi_break.yul

@@ -0,0 +1,74 @@
+{
+    let x := 1
+    let y := 1
+    let z := 1
+    for { } calldataload(0) { mstore(x, 2) mstore(z, 2) }
+    {
+        y := 3
+        switch callvalue()
+        case 0 {
+            x := 2
+            y := 2
+            z := 2
+            break
+        }
+        case 1 {
+            x := 3
+            y := 3
+            z := 3
+            continue
+        }
+        case 2 {
+            x := 4
+            y := 4
+            z := 4
+            break
+        }
+        case 3 {
+            x := 5
+            y := 5
+            z := 5
+            continue
+        }
+        mstore(y, 9)
+    }
+    mstore(x, 0x42)
+}
+// ----
+// redundantAssignEliminator
+// {
+//     let x := 1
+//     let y := 1
+//     let z := 1
+//     for {
+//     }
+//     calldataload(0)
+//     {
+//         mstore(x, 2)
+//         mstore(z, 2)
+//     }
+//     {
+//         y := 3
+//         switch callvalue()
+//         case 0 {
+//             x := 2
+//             break
+//         }
+//         case 1 {
+//             x := 3
+//             z := 3
+//             continue
+//         }
+//         case 2 {
+//             x := 4
+//             break
+//         }
+//         case 3 {
+//             x := 5
+//             z := 5
+//             continue
+//         }
+//         mstore(y, 9)
+//     }
+//     mstore(x, 0x42)
+// }

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_nested.yul

@@ -0,0 +1,76 @@
+{
+    let x := 1
+    let y := 1
+    let a := 7
+    let b := 9
+    for { } calldataload(0) { }
+    {
+        y := 9
+        mstore(a, 7)
+        if callvalue() {
+            x := 2
+            for {} calldataload(1) {}
+            {
+                a := 2 // can be removed
+                if eq(x, 3) {
+                    b := 3 // cannot be removed
+                    y := 2 // will be removed
+                    continue
+                }
+            }
+            mstore(b, 2)
+            break
+        }
+        if eq(callvalue(), 3) {
+            x := 12
+            y := 12
+            continue
+        }
+        x := 3
+        mstore(y, 3)
+    }
+    mstore(x, 0x42)
+}
+// ----
+// redundantAssignEliminator
+// {
+//     let x := 1
+//     let y := 1
+//     let a := 7
+//     let b := 9
+//     for {
+//     }
+//     calldataload(0)
+//     {
+//     }
+//     {
+//         y := 9
+//         mstore(a, 7)
+//         if callvalue()
+//         {
+//             x := 2
+//             for {
+//             }
+//             calldataload(1)
+//             {
+//             }
+//             {
+//                 if eq(x, 3)
+//                 {
+//                     b := 3
+//                     continue
+//                 }
+//             }
+//             mstore(b, 2)
+//             break
+//         }
+//         if eq(callvalue(), 3)
+//         {
+//             x := 12
+//             continue
+//         }
+//         x := 3
+//         mstore(y, 3)
+//     }
+//     mstore(x, 0x42)
+// }

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_stmnts_after_break_continue.yul

@@ -0,0 +1,52 @@
+{
+    let x := 1
+    let y := 1
+    for { } calldataload(0) { }
+    {
+        y := 9
+        if callvalue() {
+            x := 2
+            y := 2 // will be removed
+            break
+            x := 7 // after break, we start with fresh state.
+        }
+        if eq(callvalue(), 3) {
+            x := 12
+            y := 12 // will be removed
+            continue
+            x := 17 // after continue, we start with fresh state.
+            y := 9
+        }
+        x := 3
+        mstore(y, 3)
+    }
+    mstore(x, 0x42)
+}
+// ----
+// redundantAssignEliminator
+// {
+//     let x := 1
+//     let y := 1
+//     for {
+//     }
+//     calldataload(0)
+//     {
+//     }
+//     {
+//         y := 9
+//         if callvalue()
+//         {
+//             x := 2
+//             break
+//         }
+//         if eq(callvalue(), 3)
+//         {
+//             x := 12
+//             continue
+//             y := 9
+//         }
+//         x := 3
+//         mstore(y, 3)
+//     }
+//     mstore(x, 0x42)
+// }