On Wed, 20 May 2026 15:00:47 GMT, Andrew Dinn <[email protected]> wrote:
>> src/hotspot/cpu/aarch64/register_aarch64.hpp line 546:
>>
>>> 544: template<int N>
>>> 545: VSeq<N-1> vs_tail(const VSeq<N>& v) {
>>> 546: static_assert(N > 1, "tail sequence length must be greater than 2");
>>
>> Which one is it, `N > 1`, or `greater than 2`?
>
> Full disclosure -- this is my copypasta introduced during an earlier round of
> reviewing.
>
> Given that we have a static assert that `N > 2` in the definition of class
> `VSeq` I guess this is redundant. Likewise, the assertion in `vs_head`.
>
> It doesn't really make any sense to /declare/ a `VSeq` with less than 2
> elements as the purpose of the class is to provide a way to manage and access
> more than one FloatRegister using indexed notation (`vs[i]`). However, the
> provision of `vs_tail` gives rise to the possibility that we might create a
> `VSeq<1>` from a `VSeq<2>` so we could consider adjusting the asserts
> accordingly.
>
> In fact, `vs_head` and `vs_tail` were added to deal with the need for the
> current proposed kernel to operate on groups of 5 registers at a time. The
> code uses `vs_head` to access the first register in the group and `vs_tail`
> to access a sequence over the remaining four registers (n.b. calling
> `vs_head(vs)` is not strictly needed -- you can just use `vs[0]` -- so maybe
> we don't need to provide it).
>
> So, for current requirements, we could just delete the assert here and in
> `vs_head` and rely on the one in the definition of `VSeq`. If we ever need to
> split off the tail of a `VSeq<2>` we could reconsider this.
>
> Likewise we could drop `vs_head`. I thought it would be good to provide it
> because it emphasise the way that the sequence is being recursively split in
> two and ... well ... Lisp!
Dropped the assert for now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30941#discussion_r3288555091
