[PATCH] D147567: [VPlan] Only create extracts for recurrence exits if there are live-outs.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:3837
   Value *Incoming = State.get(PreviousDef, UF - 1);
   auto *ExtractForScalar = Incoming;
   auto *IdxTy = Builder.getInt32Ty();
----------------
fhahn wrote:
> Ayal wrote:
> > nit: define ExtractForScalar closer to its use below?
> > 
> > Set insert point, and possibly RuntimeVF, early for both Extract's.
> Will do separately.
Should be done in f9d0b35d2238


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:3844
     auto *LastIdx = Builder.CreateSub(RuntimeVF, One);
     ExtractForScalar = Builder.CreateExtractElement(ExtractForScalar, LastIdx,
                                                     "vector.recur.extract");
----------------
fhahn wrote:
> Ayal wrote:
> > nit: use Incoming instead of ExtractForScalar, similar to ExtractForPhiUsedOutsideLoop.
> Will do separately.
Should be done in f9d0b35d2238


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:3868
+          Incoming, Idx, "vector.recur.extract.for.phi");
+    } else if (UF > 1)
+      // When loop is unrolled without vectorizing, initialize
----------------
fhahn wrote:
> Ayal wrote:
> > nit: should this `else if (UF > 1)` be an `else { assert(UF > 1 && "VF and UF are both 1?"); ...}`?
> > 
> Looks like it, I'll turn it into an assert separately.
Should be done in 954befe2a745


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147567