[PATCH] D32737: [Constants][SVE] Represent the runtime length of a scalable vector

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

In https://reviews.llvm.org/D32737#760430, @lattner wrote:

> > From my point of view, making scalable vectors a native and core type of IR is the only way forward, because the semantics needs to be ingrained in the language to make sense. AFAICS, at the IR level, the differences between vectors and scalable ones is not that big a deal, certainly not bigger than vectors versus scalar.
>
> This is much more up for debate.  Details matter.  Compilers are a set of engineering tradeoffs.  You need to explain and defend your position carefully, not just state it as though it were "obviously true".


I though that was clear from my previous response to you: "I don't think anyone disagrees with that point."

I'm certainly not asking people to "trust me, I'm an engineer". We had previous discussions in the list, other people chimed in. This is not even my patch and I have nothing to do with their work... "I just work here".

I'm also certainly not asserting that I know all the answers and that this change is worry free, by any means, and that's precisely why I pinged more people to give their opinions, because I was uncomfortable with the low amount of reviews. But it was certainly not "just me".

> Do you have a discussion somewhere of the overall design of the extensions you're proposing?  This is presumably only a small piece of it.

There were discussions on the list, a "whole-patch approach" in Github and some previous patches. I can't find any of it (my mail client - gmail - is a mess). I'll let Graham cover that part.

> You haven't proposed a set of IR type system extensions.  If you're proposing a new first class type, one which represents a vector of an unknown length, then you'll need to be able to load and store it, e.g. to spill a virtual register.  How much stack space is required for that spill?

Ah, right. I'll let Graham take that one, as they have done this already in LLVM.

> Seriously, I happen to be very familiar with the hardware/implementation model of this instruction set extension.

I meant no offence.

> The thing that matters here is the specific set of IR extensions you're proposing.  In this patch you're proposing introducing a vscale constant.

s/me/Graham/, this is not my patch and I had no part in any of this.

> This doesn't make sense, because it cannot be used in (e.g.) a global variable initializer, and otherwise doesn't fit the model for constants.  Why not use an intrinsic to return this value?

IIUC, it can be used for global variable initialiser via SVE splats using another new construct (stepvector) they want to introduce (which I had my own concerns).

I think the "stepvector" idea is limiting and could possibly start as an intrinsic, but the vscale is not really the actual scale, just a flag that it is scalable, so it wouldn't have more problems than using some intrinsic.

It'll still need a new register class in ISel and the register allocation, and it will still need to understand the aliasing rules, in the same way as we currently have for VFP and NEON.

If the vscale could make the stack size variable, so would an intrinsic. Or maybe I'm just not understanding the problem.

cheers,
--renato


https://reviews.llvm.org/D32737