[PATCH] D147259: [LV] Don't sink scalar instructions that may read from memory.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added a comment.

In D147259#4272365 <https://reviews.llvm.org/D147259#4272365>, @Ayal wrote:

> Looks good to me, thanks for fixing! Worth mentioning that this fixes a bug?

Thanks! There's no bug report I am aware of, but the commit message mentions that this avoids a mis-compile.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4169
       // We can't sink an instruction if it is a phi node, is not in the loop,
       // or may have side effects.
       if (!I || isa<PHINode>(I) || !VectorLoop->contains(I) ||
----------------
Ayal wrote:
> nit: // ... or may read from memory.
> 
> Worth adding a TODO to allow sinking a load past non-store instructions? (Could potentially also sink obstructing stores as well(?)) Better during VPlan-to-VPlan sinking.
Thanks, added the comment + TODO.

> Worth adding a TODO to allow sinking a load past non-store instructions? (Could potentially also sink obstructing stores as well(?)) Better during VPlan-to-VPlan sinking.

Agreed, possibly re-use the same analysis as for sinking memory instructions in the VPlan-based recurrence handling code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147259