RFR: 8351623: VectorAPI: Add SVE implementation of subword gather load operation [v5]

[Emanuel Peter]

On Fri, 5 Sep 2025 10:37:39 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Xiaohong Gong has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
>> 
>>  - Merge 'jdk:master' into JDK-8351623-sve
>>  - Address review comments
>>  - Refine IR pattern and clean backend rules
>>  - Fix indentation issue and move the helper matcher method to header files
>>  - Merge branch jdk:master into JDK-8351623-sve
>>  - 8351623: VectorAPI: Add SVE implementation of subword gather load operation
>
> src/hotspot/share/opto/vectornode.hpp line 1123:
> 
>> 1121:   // The basic type of memory, which might be different with the vector element type
>> 1122:   // when it is a subword type loading.
>> 1123:   BasicType _mem_bt;
> 
> Can you make an example and add it to the comment?
> Can you please also add some comment at the node about what we expect the index map to be? What basic type does it have?

Same for the scatter.

> src/hotspot/share/opto/vectornode.hpp line 1769:
> 
>> 1767: //      dst = [h g f e d c b a]
>> 1768: //
>> 1769: class VectorConcatenateNode : public VectorNode {
> 
> That semantic is not quite what I would expect from `Concatenate`. Maybe we can call it something else?
> `VectorConcatenateAndNarrowNode`?

Have you considered using `2x Cast + Concatenate` instead, and just matching that in the backend? I don't remember how to do the mere Concat, but it should be possible via the `unslice` or some other operation that concatenates two vectors.

> src/hotspot/share/opto/vectornode.hpp line 1774:
> 
>> 1772:     : VectorNode(vec1, vec2, vt) {
>> 1773:     assert(type2aelembytes(vec1->bottom_type()->is_vect()->element_basic_type()) ==
>> 1774:            type2aelembytes(vt->element_basic_type()) * 2, "must be half size");
> 
> What about asserting that `vec1` and `vec2` have the same `vect`?

And what about the vector length being consistent between `vec1`, `vec2` and `vt`?

> src/hotspot/share/opto/vectornode.hpp line 1841:
> 
>> 1839: 
>> 1840: // Unpack the elements to twice size.
>> 1841: class VectorMaskWidenNode : public VectorNode {
> 
> Can you add a visual example like above for `VectorConcatenateNode`, please?

Did you consider the alternative of `Extract` + `Cast`? Not sure if that would be better, you know more about the code complexity. It would just allow us to have one fewer nodes.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26236#discussion_r2324737096
PR Review Comment: https://git.openjdk.org/jdk/pull/26236#discussion_r2324754984
PR Review Comment: https://git.openjdk.org/jdk/pull/26236#discussion_r2324742727
PR Review Comment: https://git.openjdk.org/jdk/pull/26236#discussion_r2324748722