[PATCH] D32530: [SVE][IR] Scalable Vector IR Type

[Graham Hunter via Phabricator via llvm-commits]

huntergr added inline comments.


================
Comment at: docs/LangRef.rst:2749
+of size zero are not allowed. For scalable vectors, the number of
+elements is an unknown integer multiple of the number of elements.
 
----------------
rkruppe wrote:
> rengolin wrote:
> > hfinkel wrote:
> > > This doesn't seem strong enough. We need the unknown multiple to be the *same* for any given type (at least within a given function).  We also need a relationship between vectors of different underlying types (so that zext/sext/etc. make sense). Otherwise, you can't even sensibly add them together (for example). I realize that it says something about an unknown vector length above, but we need to translate that statement into semantics that make sense for the vectors themselves.
> > It's not that simple. Both SVE and RISC-V can have vector multiplier changes in the middle of a function (via system register or similar). Neither of them *want* that to be the norm, but IIRC, RISC-V doesn't want it to change inside a function and SVE wants it to be the same for the whole program.
> > 
> > I totally agree with you that leaving it open is a huge can of worms, and wanting a per-function change would probably need new annotation on functions, which if ever done, should be orthogonal to this change (or would lead us into madness).
> > 
> > I second your proposal that we fix the semantics in LLVM, for now, that the "unknown width" is the same throughout the program and that the existing relationship between fixed vectors extends to scalable vectors.
> > 
> > If you look at the changes in this patch series, it assumes that behaviour already, by getting new vector types of half-size with double-elements and so on.
> > 
> > IFF RISC-V wants to extend the logic to be per-function, then we will need to do a much more extensive analysis on the passes, especially around inlining and function calls. I strongly propose we don't look at it right now and fix the semantics as proposed above.
> > 
> > In my analysis, with that semantics, I don't see a big impact on any existing non-scalable optimisations. With vectorisation passes being run at the end of the pipeline, even for scalable code, most of the existing pipeline will still be relevant, too.
> So first of all, I agree that this patch does (and should) only implement a single "constant" (at runtime) `vscale` value. The current wording here in LangRef is ambiguous about this, it doesn't make clear at which scope the "unknown integer multiple" is fixed. It should be made clear that this factor is the same for all vector types and does not change while the program executes (and, once the `vscale` intrinsic is added, this section here should also point at it).
> 
> Second, since you mentioned it I should say: the RISC-V vector extension has now changed to a point where (at least in its standard incarnation, without further extensions on top of it) there is no need for vscale to change during runtime, let alone between individual functions. All the changes to the "vector register size" now happen in such a way that it's easy to express the longer vectors in IR by just using different vector types with higher `ElementCount::Min`, e.g. `<scalable 8 x double>` instead of `<scalable 1 x double>`. So from the RVV side, there's no need any more for `vscale` to vary e.g. function-by-function.
> 
> I don't know whether the SVE folks want to take a shot at it regardless, but in past discussion it sounded like the vscale-per-function model wasn't a good fit for the programming model they envisioned, so maybe they'd come up with a different solution if and when they tackle that problem.
For SVE at least, we can consider changing the vector length during execution to be undefined behaviour. The compiler is not expected to be able to handle it.

For RVV, given the new restrictions on how vlmul is handled, I think they won't need to change the multiple at runtime either -- just increase the the minimum number of lanes. I'm hoping to discuss this with Robin at EuroLLVM, assuming time permits.

I'll come up with some stricter wording.


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

https://reviews.llvm.org/D32530