[PATCH] D154309: [LV] Do not add load to group if it moves across conflicting store.

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: llvm/lib/Analysis/VectorUtils.cpp:1175
       // the boundaries of the (2, 4) group.
       if (!canReorderMemAccessesForInterleavedGroups(&*AI, &*BI)) {
         // If a dependence exists and A is already in a group, we know that A
----------------
Ayal wrote:
> Ayal wrote:
> > Sketch of one option to fix insertion of Group into CompletedLoadGroups whenever/as-soon-as needed.
> Anna, here's a more concrete sketch, which also breaks out of the enclosing "for AI" loop as needed:
> 
> ```
>       if (Group && isa<LoadInst>(B)) {
>         uint32_t Index = 0, Factor = Group->getFactor();
>         for (; Index < Factor; ++Index) {
>           Instruction *MemberOfGroupB = Group->getMember(Index);
>           if (MemberOfGroupB &&
>               !canReorderMemAccessesForInterleavedGroups(
>                   &*AI, &*AccessStrideInfo.find(MemberOfGroupB)))
>             break;
>         }
>         if (Index < Factor) {
>           CompletedLoadGroups.insert(Group);
>           break;
>         }
>       }
> ```
> Curious to learn if it helps.
thanks Ayal! My fix had missed that we need to break out of the "for AI" loop (the original fix was automatically doing that when we check for `canReorderMemAccessesForInterleavedGroups` for single `BI` at line 1208).
With this the modified testcase you suggested optimizes correctly.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154309