[PATCH] D50579: [LV] Teach about non header phis that have uses outside the loop

[Anna Thomas via Phabricator via llvm-commits]

anna marked 2 inline comments as done.
anna added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3724
+      auto *IncomingValue = LCSSAPhi.getIncomingValue(0);
+      if (!Legal->isAllowedNonHeaderExit(IncomingValue)) {
+        assert(OrigLoop->isLoopInvariant(LCSSAPhi.getIncomingValue(0)) &&
----------------
Ayal wrote:
> Distinguishing between different `AllowedExits` here seem redundant: if `IncomingValue` `isLoopInvariant()`, as asserted below, `getOrCreateScalarValue({UF-1,VF-1})` will simply return it. So the 'else' case below can also take care of the 'then' case. In general, whatever the allowed exit may be, grab the scalar value associated with the last iteration.
> 
> This will make `AllowedNonHdrExits` useless.
> 
> In the most general form, one may need to do
> ```
>   unsigned LastLane = Cost->isUniformAfterVectorization(IncomingValue, VF) ? 0 : VF - 1;
>   Value *lastIncomingValue =
>             getOrCreateScalarValue(IncomingValue, {UF - 1, LastLane});
> ```
> but this is not needed for this patch.
good point. will reduce complexity here.


================
Comment at: test/Transforms/LoopVectorize/no_outside_user.ll:207
+; cyclic dependency with induction var
+; cannot vectorize.
+; CHECK-LABEL: cyclic_dep_with_indvar(
----------------
Ayal wrote:
> In `cyclic_dep_with_indvar()` below, `%iv` is conditionally reset inside the loop, disqualifying it from being an induction variable; so the loop isn't vectorizable, regardless of live-outs. Right? 
yes, I had added the test case to show that cyclic dependencies that do not make the loop vectorizable still remain non-vectorizable. Same behaviour with not allowable cyclic dependencies for reductions. 

I don't think there's a test where the loop was vectorizable in absence of live-outs and now is no longer vectorizable because of the live-outs. That's what this patch fixes :)



Repository:
  rL LLVM

https://reviews.llvm.org/D50579