[PATCH] D93629: [LV] Don't sink into replication regions

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8610
+        Sink->removeFromParent();
+        NextBlock->insert(Sink, NextBlock->begin());
+        continue;
----------------
fhahn wrote:
> fhahn wrote:
> > can this just be `Sink->moveBefore(NextBlock->getFirstInsertionPt())`? With that, `Sink->removeFromParent()` shouldn't be needed?
> Oh right, `moveBefore` was still missing. Thanks for adding it. I'm not sure if there's a benefit of having the variant that takes a basic block & an iterator? For consistency with the existing functions, I think it would be slightly better to just have the version that moves before a `VPRecipeBase` (same as `moveAfter`)? Those versions are easier & more convenient to use, IMO.
It was added because the iterator might be end(), in which case it's not possible to tell which block it originally referred to. (The block might be empty, that's why it can't use moveAfter). It is modelled after this equivalent method, from BasicBlock/Instruction: https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/IR/Instruction.h#L142


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

https://reviews.llvm.org/D93629