[PATCH] D69228: [LV] Generalize conditions for sinking instrs for first order recurrences.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 4 inline comments as done.
fhahn added a comment.

Thanks Ayal, committed with the comments addressed.



================
Comment at: llvm/lib/Analysis/IVDescriptors.cpp:728
+
+    if (I->getParent() == Phi->getParent() && canSinkAfter) {
+      SinkAfter[I] = Previous;
----------------
Ayal wrote:
> It may be better to check allUsesDominatedBy() last, i.e., after checking for same Parents, in terms of compile-time.
I've moved the checks in the if directly, no need to trust the compiler to sink the checks.


================
Comment at: llvm/test/Transforms/LoopVectorize/first-order-recurrence-complex.ll:121
+define void @cannot_sink_with_additional_user(i32 %x, i32* %ptr, i64 %tc) {
+; CHECK-LABEL: define void @cannot_sink_with_additional_user(
+; CHECK-NEXT: entry:
----------------
Ayal wrote:
> Alternatively CHECK-NOT that no vector type got generated? Holds for additional not-to-be-vectorized tests below.
I've kept the more verbose check lines, as it seems a bit more explicit (and they are still quite compact IMO)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69228