[lworld+vector] RFR: 8311675: [lworld+vector] Max Species support.

[Xiaohong Gong]

On Mon, 9 Oct 2023 02:43:21 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> src/hotspot/share/classfile/classFileParser.cpp line 1636:
>> 
>>> 1634:       int bundle_size = parsed_annotations.multifield_arg();
>>> 1635:       if (bundle_size < 0) {
>>> 1636:         bundle_size = VectorSupport::get_max_multifield_count(_class_name);
>> 
>> An alternative is trying to passing the type info to the `MultiField` annotation, so that the `_class_name` is not needed, and so does to the method `VectorSupport::get_max_multifield_count()`. We can use "max_vector_size(bt)" directly.
>
>> An alternative is trying to passing the type info to the `MultiField` annotation, so that the `_class_name` is not needed, and so does to the method `VectorSupport::get_max_multifield_count()`. We can use "max_vector_size(bt)" directly.
> 
> Thanks @XiaohongGong , you mean pass explicit vector lane type to multifiled annotation like @multifield(-1,  long.class). Please consider following points 
> - We will still need explicit VectorPayload class definitions for mask/shuffule max payloads for different vector types.
> - Mask/Shuffle backing storage field type is common for all kinds of vector i.e. byte for shuffle and Boolean for mask.
> - We can save on complexity of extending the annotation semantics for max species by doing an efficient vmSymbolID lookup. 
> - In future if we refactor shuffle lanes to match the lane types of corresponding vector then additional element type in multifield will be redundant as the backing field type will itself be different for max shuffle payloads.

I understand. It's not a problem for mask/shuffle, since we can add the type info for them as well. And I think that's the main reason that we need the type info. Because we can get the type info during class parser for each field of the normal vector classes. Anyway, looking up symbol ID can also work well.

>> src/hotspot/share/prims/vectorSupport.cpp line 493:
>> 
>>> 491: int VectorSupport::max_vector_size(BasicType bt) {
>>> 492:   if (is_java_primitive(bt)) {
>>> 493:     return VM_Version::max_vector_size(bt);
>> 
>> `VM_Version::max_vector_size(bt)` is a platform specific method, and currently only x86 contains it. So this makes the jdk image build failure on other platforms like aarch64. Could we define a public/common method and implement it on each platform like matcher?
>
> Correct, we need handle it for aarch64.  Matcher routines will not be accessible by C1 only builds. That was a limitation in existing [_VectorSupport_GetMaxLaneCount_](https://github.com/openjdk/valhalla/blob/lworld/src/hotspot/share/prims/vectorSupport.cpp#L583) functionality. Max vector size is tightly bound to targets.
> 
>> Could we define a public/common method and implement it on each platform like matcher
> 
> Yes, x86 already does that currently, we need to have a target specific handling one way or other, We need need max vector size to estimate bundle size during field parsing and wanted to avoid its dependency on opto compiler.

Yeah, I agree that we need a non-c2 specific method to get the max vector size. But it is added for x86 now. And I can help add the aarch64 part if you need. My concern is for other platforms that do not have such a method, the jdk image will build failure.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/931#discussion_r1349965724
PR Review Comment: https://git.openjdk.org/valhalla/pull/931#discussion_r1349962339