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

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2807
+        PartPtr->setIsInBounds(InBounds);
+      } else {
+        PartPtr = cast<GetElementPtrInst>(
----------------
bin.cheng-ali wrote:
> Sorry for one nitpicking.
> IIUC the difference for scalable/fixed cases is the two operands of GEPs, is it better to factor out common code by doing below?
> if (VF.isScalable()) {
>   // build GEP operands for scalable case
> }
> else {
>   // build GEP operands for fixed case
> }
> // common code building VecPtr with above GEP operands.
I guess alternatively we could commonise them into one block where we always calculate

```        Value *LastLane =
            Builder.CreateSub(Builder.getInt32(1),
                              getRuntimeVF(Builder, Builder.getInt32Ty(), VF));```

for both fixed-width and scalable vectors, and just let instcombine and other passes fold the constants? I'm not sure if that breaks lots of tests though ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95363