[PATCH] D95363: [SVE][LoopVectorize] Add support for scalable vectorization of loops with vector reverse

[David Sherwood via Phabricator via llvm-commits]

david-arm added a comment.

Hi @CarolineConcatto, it's looking much better now and thanks for dealing with all the review comments!



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2818
       if (isMaskRequired) // Reverse of a null all-one mask is a null mask.
         BlockInMaskParts[Part] = reverseVector(BlockInMaskParts[Part]);
     } else {
----------------
Hi @CarolineConcatto, I just realised there isn't a test for this for scalable vectors. Would you be able to add a test for blocks with masks too? I think it should be something like:

```void foo(int * __restrict__ a, int * __restrict__ cond, long long N) {
  for (int i = N - 1; i >= 0; i--)
  {
    if (cond[i])
      a[i] += 1;
  }
}```



================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/vector-reverse.ll:22
+; CHECK-NEXT: %[[WIDE:.*]] = load <8 x double>, <8 x double>* %[[CAST]], align 8
+; CHECK-NEXT: %[[REVERSE:.*]] = shufflevector <8 x double> %[[WIDE]]
+; CHECK-NEXT: %[[FADD:.*]] = fadd <8 x double> %[[REVERSE]]
----------------
I think that since you're adding tests for this it's probably a good idea to expand all the shufflevector CHECK lines in this file to make sure we have a reverse mask here too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95363