[PATCH] D95139: [SVE][LoopVectorize] Add support for extracting the last lane of a scalable vector

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4428
+      else
+        Lane = VF.getKnownMinValue() - 1;
+    }
----------------
sdesmalen wrote:
> david-arm wrote:
> > sdesmalen wrote:
> > > Should Lane not be set to VF.getKnownMinValue() - 1 for VF.isScalable() as well?
> > Not in the way I've defined it, i.e. LastLane - Lane. This is a bit similar to how the offset is defined for Cullen's vector.splice intrinsic. A Lane of 0 is the same as LastLane - this does make the calculation of the runtime lane a bit easier.
> What I don't like about that definition is that the representation in the LoopVectorizer is entirely different for scalable and fixed-width vectors. `{0, First}` and `{0, LastScalable}` mean the first and last element, respectively. `{3, First}` means the last for `<4 x i32>` and `{3, LastScalable}` means 4th last element (for `<vscale x 4 x i32>`). I think this is confusing and error prone. If First/ScalableLast would mean the first and last scalable "chunk" of a vector, with the index being the index as normal, then the last iteration would be described as `{3, First}` for `<4 x i32>` and `{3, ScalableLast}`  for `<vscale x 4 x i32>`, and so the representation in the LoopVectorizer is more or less the same. Code-generating it is slightly different of course, but there's probably only a single place where that has to happen.
OK, I'll have a think about it and see code changes would be required. I think in terms of code-generating the lane index, starting from the end is easier for the developer as the most common case will be the very last lane. That's why in my original patch I'd only catered specifically for the very last lane. If we define it in terms of chunks (really just subvectors I think) I might change some of the interfaces and names to make it clear that we're dealing with lane indices from the start of what is essentially a subvector.


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

https://reviews.llvm.org/D95139