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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8610
+        Sink->removeFromParent();
+        NextBlock->insert(Sink, NextBlock->begin());
+        continue;
----------------
can this just be `Sink->moveBefore(NextBlock->getFirstInsertionPt())`? With that, `Sink->removeFromParent()` shouldn't be needed?


================
Comment at: llvm/test/Transforms/LoopVectorize/first-order-recurrence.ll:650
+; CHECK-LABEL: sink_into_replication_region
+; CHECK: <4 x i32>
+define i32 @sink_into_replication_region() {
----------------
It's probably worth to check the IR for the whole vector body for the sink-after case.


================
Comment at: llvm/test/Transforms/LoopVectorize/first-order-recurrence.ll:664
+  %tmp6 = add i32 %tmp5, %tmp4
+  %tmp7 = udiv i32 219220132, %tmp3
+  %tmp8 = add nsw i32 %tmp3, -1
----------------
Can you also add another instruction that will get predicate? I think currently things will work out as expected, because we predicate each instruction separately, but it could lead to problems if we predicate multiple instructions together and cannot sink to the right position. But this will get re-worked as part of the VPlan transition, so I think a test case to catch this should be sufficient.


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

https://reviews.llvm.org/D93629