[PATCH] D100751: [VPlan] Properly handle sinking of replicate regions.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.

This looks good, thanks for fixing, adding a couple of nits.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8966
     VPRecipeBase *Sink = RecipeBuilder.getRecipe(Entry.first);
     VPRecipeBase *Target = RecipeBuilder.getRecipe(Entry.second);
     // If the target is in a replication region, make sure to move Sink to the
----------------
> I think we also need to deal with the case when the target is a
> replicate region also. I'll put up a patch once I constructed a test
> for that scenario.

Agreed. Worth adding an assert in the meanwhile?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9005
+        VPBB = SplitBlock;
+      continue;
+    }
----------------
better early-exit the simple `Sink->moveAfter(Target)` non-pred-rep case first?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:407
+VPBasicBlock *VPBasicBlock::splitAt(iterator SplitAt) {
+  assert(SplitAt->getParent() == this);
+
----------------
error message


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1513
+  /// Split current block at \p SplitAt by inserting a new block between the
+  /// current block and its successors and mobing all recipes starting at
+  /// SplitAt to the new block. Returns the new block.
----------------
mobing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100751/new/

https://reviews.llvm.org/D100751