[PATCH] D111301: [VPlan] Make sure recurrence splice is not inserted between phis.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.

In D111301#3112774 <https://reviews.llvm.org/D111301#3112774>, @fhahn wrote:

> In D111301#3109192 <https://reviews.llvm.org/D111301#3109192>, @Ayal wrote:
>
>> In D111301#3108536 <https://reviews.llvm.org/D111301#3108536>, @fhahn wrote:
>>
>>> ping :)
>>>
>>> @Ayal does the clarification why this is needed make sense?
>>>
>>>> (from inline comment)
>>>> Ok I now what phi-like case I originally hit: if the previous value is an induction value! So I think we still need the special handling?
>>
>> Clarification indeed makes sense, induction feeding a FOR should presumably be handled, not being a 2nd-order recurrence (SOR).
>>
>> So we may indeed still need the special handling - along with special test(s); perhaps using i-1 inside a "for i" loop?
>
> There should be test coverage already for those cases, in `Transforms/LoopVectorize/induction.ll`, which fails verification from D111302 <https://reviews.llvm.org/D111302> without the current patch.

ok; can a comment be added to these tests, mentioning that they exercise induction-feeding-a-FOR, and careful placement of the FOR splice?
Wonder how VPlan generated correct code, with splice placed in the midst of phi's(?)

>> Would also be good to address/exclude/cover non-header phis - of replicated regions and blends.
>
> Should those be included in this patch here?

or left as a TODO?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9456
       continue;
 
+    VPRecipeBase *PrevRecipe = RecurPhi->getBackedgeRecipe();
----------------
nit: can tentatively set

```
  VPBasicBlock *InsertBlock = PrevRecipe->getParent();
```
initially, and later override it with

```
  InsertBlock = cast<VPBasicBlock>(Region->getSingleSuccessor());
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111301