Merge pull request #6568 from ethereum/fixDeepFor

Disable redundent assign eliminator for deeply nested loops.

libyul/optimiser/RedundantAssignEliminator.cpp

@@ -117,13 +117,13 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 {
 	ForLoopInfo outerForLoopInfo;
 	swap(outerForLoopInfo, m_forLoopInfo);
+	++m_forLoopNestingDepth;
 
 	// If the pre block was not empty,
 	// we would have to deal with more complicated scoping rules.
 	assertThrow(_forLoop.pre.statements.empty(), OptimizerException, "");
 
-	// We just run the loop twice to account for the
+	// We just run the loop twice to account for the back edge.
-	// back edge.
 	// There need not be more runs because we only have three different states.
 
 	visit(*_forLoop.condition);
@@ -137,24 +137,46 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 
 	visit(*_forLoop.condition);
 
-	TrackedAssignments oneRun{m_assignments};
+	if (m_forLoopNestingDepth < 6)
+	{
+		// Do the second run only for small nesting depths to avoid horrible runtime.
+		TrackedAssignments oneRun{m_assignments};
 
-	(*this)(_forLoop.body);
+		(*this)(_forLoop.body);
 
-	merge(m_assignments, move(m_forLoopInfo.pendingContinueStmts));
+		merge(m_assignments, move(m_forLoopInfo.pendingContinueStmts));
-	m_forLoopInfo.pendingContinueStmts.clear();
+		m_forLoopInfo.pendingContinueStmts.clear();
-	(*this)(_forLoop.post);
+		(*this)(_forLoop.post);
 
-	visit(*_forLoop.condition);
+		visit(*_forLoop.condition);
+		// Order of merging does not matter because "max" is commutative and associative.
+		merge(m_assignments, move(oneRun));
+	}
+	else
+	{
+		// Shortcut to avoid horrible runtime:
+		// Change all assignments that were newly introduced in the for loop to "used".
+		// We do not have to do that with the "break" or "continue" paths, because
+		// they will be joined later anyway.
+		// TODO parallel traversal might be more efficient here.
+		for (auto& var: m_assignments)
+			for (auto& assignment: var.second)
+			{
+				auto zeroIt = zeroRuns.find(var.first);
+				if (zeroIt != zeroRuns.end() && zeroIt->second.count(assignment.first))
+					continue;
+				assignment.second = State::Value::Used;
+			}
+	}
 
-	// Order does not matter because "max" is commutative and associative.
+	// Order of merging 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();
 
 	// Restore potential outer for-loop states.
 	swap(m_forLoopInfo, outerForLoopInfo);
+	--m_forLoopNestingDepth;
 }
 
 void RedundantAssignEliminator::operator()(Break const&)

libyul/optimiser/RedundantAssignEliminator.h

@@ -82,6 +82,11 @@ struct Dialect;
  * one run and two runs and then combine them at the end.
  * Running at most twice is enough because there are only three different states.
  *
+ * Since this algorithm has exponential runtime in the nesting depth of for loops,
+ * a shortcut is taken at a certain nesting level: We only use the zero- and
+ * once-run of the for loop and change any assignment that was newly introduced
+ * in the for loop from to "used".
+ *
  * For switch statements that have a "default"-case, there is no control-flow
  * part that skips the switch.
  *
@@ -168,6 +173,7 @@ private:
 		std::vector<TrackedAssignments> pendingContinueStmts;
 	};
 	ForLoopInfo m_forLoopInfo;
+	size_t m_forLoopNestingDepth = 0;
 };
 
 class AssignmentRemover: public ASTModifier

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_deep_noremove.yul

@@ -0,0 +1,88 @@
+{
+    let x := 1
+    let y := 2
+    let a := 3
+    let b := 4
+    for {} 1 {} {
+    for {} 1 {} {
+    for {} 1 {} {
+    // Here, the nesting is not yet too deep, so the assignment
+    // should be removed.
+    for {} 1 { x := 9 } {
+        y := 10
+        for {} 1 {} {
+        for {} 1 {} {
+        // Now we are too deep, assignments stay.
+        for {} 1 { a := 10 } {
+            b := 12
+            b := 11
+        }
+        }
+        }
+    }
+    }
+    }
+    }
+    x := 13
+}
+// ====
+// step: redundantAssignEliminator
+// ----
+// {
+//     let x := 1
+//     let y := 2
+//     let a := 3
+//     let b := 4
+//     for {
+//     }
+//     1
+//     {
+//     }
+//     {
+//         for {
+//         }
+//         1
+//         {
+//         }
+//         {
+//             for {
+//             }
+//             1
+//             {
+//             }
+//             {
+//                 for {
+//                 }
+//                 1
+//                 {
+//                 }
+//                 {
+//                     for {
+//                     }
+//                     1
+//                     {
+//                     }
+//                     {
+//                         for {
+//                         }
+//                         1
+//                         {
+//                         }
+//                         {
+//                             for {
+//                             }
+//                             1
+//                             {
+//                                 a := 10
+//                             }
+//                             {
+//                                 b := 12
+//                                 b := 11
+//                             }
+//                         }
+//                     }
+//                 }
+//             }
+//         }
+//     }
+// }

test/libyul/yulOptimizerTests/redundantAssignEliminator/for_deep_simple.yul

@@ -0,0 +1,76 @@
+{
+    for {} 1 {} {
+    for {} 1 {} {
+    for {} 1 {} {
+    for {} 1 {} {
+    for {} 1 {} {
+    for {} 1 {} {
+    for {} 1 { let a := 1 a := 2 a := 3 } {
+        // Declarations inside body and post should be handled as normal.
+        let b := 1
+        b := 2
+        b := 3
+    }
+    }
+    }
+    }
+    }
+    }
+    }
+}
+// ====
+// step: redundantAssignEliminator
+// ----
+// {
+//     for {
+//     }
+//     1
+//     {
+//     }
+//     {
+//         for {
+//         }
+//         1
+//         {
+//         }
+//         {
+//             for {
+//             }
+//             1
+//             {
+//             }
+//             {
+//                 for {
+//                 }
+//                 1
+//                 {
+//                 }
+//                 {
+//                     for {
+//                     }
+//                     1
+//                     {
+//                     }
+//                     {
+//                         for {
+//                         }
+//                         1
+//                         {
+//                         }
+//                         {
+//                             for {
+//                             }
+//                             1
+//                             {
+//                                 let a := 1
+//                             }
+//                             {
+//                                 let b := 1
+//                             }
+//                         }
+//                     }
+//                 }
+//             }
+//         }
+//     }
+// }