[PATCH] D119078: [LAA, LV] Add initial support for pointer-diff memory checks.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 406940.
fhahn added a comment.



In D119078#3302863 <https://reviews.llvm.org/D119078#3302863>, @efriedma wrote:

> If I'm following this correctly, this is basically doing two things.  One, it reduces the minimum distance by basing the required distance on the vector factor, instead of the start/end of the whole loop.  Two, it takes advantage of the ordering of different operations: a false dependency is okay as long as you perform the operations in the correct order.

That's a great summary, thanks! Those 2 things do not need to be done in a single step/patch. It is also possible (and simpler) to start with 2) - taking advantage of the ordering. I updated the patch to *only* take advantage of the ordering to emit one of the existing checks we generate (instead of 2). (This was also suggested by @Ayal offline)

The latest version of the patch emits the following check for each (src, sink) pair (if they be classified): there is a conflict if lower bound of source < upper bound of sink, i.e. if src starts before sink ends.

Changing the checks to use the pointer-difference approach can be done as follow up and has the benefit Eli mentioned, as well as removing the requirement to have computable bounds.

> It looks like we end up rejecting at runtime if the two pointers are exactly equal; if I'm following correctly, that's an unnecessary restriction?

Yes that's an unnecessary restriction of the original patch.

> Does the code generator actually guarantee that we perform operations in the correct order?  Does interleaving matter?  Other transforms?

The distance needs to also include the interleave count, if the vectorizer interleaves the vector loop. I think we also cannot add !noalias metadata with the lightweight checks, but that should guarantee that other transformations work with the correct aliasing assumptions.

AFAICT we do not rely on the noalias property in any part of the vectorizer codegenerator at the moment, but there is a patch in flight that does (D110235 <https://reviews.llvm.org/D110235>). We need to be careful there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119078

Files:
  llvm/include/llvm/Analysis/LoopAccessAnalysis.h
  llvm/include/llvm/Transforms/Utils/LoopUtils.h
  llvm/lib/Analysis/LoopAccessAnalysis.cpp
  llvm/lib/Transforms/Utils/LoopUtils.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/test/Transforms/LoopVectorize/AArch64/sve-vector-reverse.ll
  llvm/test/Transforms/LoopVectorize/ARM/mve-qabs.ll
  llvm/test/Transforms/LoopVectorize/X86/masked_load_store.ll
  llvm/test/Transforms/LoopVectorize/X86/pr23997.ll
  llvm/test/Transforms/LoopVectorize/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/multiple-exits-versioning.ll
  llvm/test/Transforms/LoopVectorize/no_outside_user.ll
  llvm/test/Transforms/LoopVectorize/runtime-check-readonly.ll
  llvm/test/Transforms/LoopVectorize/runtime-check-small-clamped-bounds.ll
  llvm/test/Transforms/LoopVectorize/runtime-check.ll
  llvm/test/Transforms/LoopVectorize/scalable-loop-unpredicated-body-scalar-tail.ll
  llvm/test/Transforms/LoopVectorize/tbaa-nodep.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D119078.406940.patch
Type: text/x-patch
Size: 252656 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220208/d0daa988/attachment-0001.bin>