[llvm] [VPlan] Process combineRecipes in a worklist (PR #213899)

[David Sherwood via llvm-commits]

================
@@ -1696,21 +1696,33 @@ static VPSingleDefRecipe *combineRecipe(VPlan &Plan, VPSingleDefRecipe *Def) {
 }
 
 void VPlanTransforms::combineRecipes(VPlan &Plan) {
-  ReversePostOrderTraversal<VPBlockDeepTraversalWrapper<VPBlockBase *>> RPOT(
+  SmallVector<VPSingleDefRecipe *, 256> Worklist;
+  PostOrderTraversal<VPBlockDeepTraversalWrapper<VPBlockBase *>> POT(
       Plan.getEntry());
-  for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(RPOT)) {
-    for (VPRecipeBase &R : make_early_inc_range(*VPBB))
+  for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(POT))
+    for (VPRecipeBase &R : reverse(*VPBB))
       if (auto *Def = dyn_cast<VPSingleDefRecipe>(&R))
-        if (VPSingleDefRecipe *New = combineRecipe(Plan, Def)) {
-          if (New != Def) {
-            // Replace the recipe with a new one.
-            Def->replaceAllUsesWith(New);
-            Def->eraseFromParent();
-          } else if (vputils::isDeadRecipe(R)) {
-            // Recipe was modified - it may be dead now.
-            Def->eraseFromParent();
-          }
-        }
+        Worklist.push_back(Def);
+
+  [[maybe_unused]] unsigned InitWorklistSize = Worklist.size();
+
+  while (!Worklist.empty()) {
+    assert(Worklist.size() < InitWorklistSize * 2 &&
+           "Worklist is growing large, possible cycle?");
+    VPSingleDefRecipe *Def = Worklist.pop_back_val();
+    VPSingleDefRecipe *New = combineRecipe(Plan, Def);
+    if (!New)
+      continue;
+    if (New != Def) {
+      // Replace the recipe with a new one.
+      Def->replaceAllUsesWith(New);
+      Def->eraseFromParent();
+      Worklist.push_back(New);
+      // TODO: Append users to the worklist (might need a setvector)
+    } else if (vputils::isDeadRecipe(*Def)) {
+      // Recipe was modified - it may be dead now.
+      Def->eraseFromParent();
+    }
----------------
david-arm wrote:

I just had a thought. Given we're now going down the path of being more like instcombine, would it be good idea to have a similar check to instcombine that when we have an assert build we ensure the second iteration of calling combineRecipes gives the same result as the first? If this happens in instcombine we emit this error:

```
LLVM ERROR: Instruction Combining on foo did not reach a fixpoint after 1 iterations.
```


https://github.com/llvm/llvm-project/pull/213899