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

Xiaohong Gong xgong at openjdk.org
Mon Oct 16 01:58:44 UTC 2023


On Sun, 15 Oct 2023 16:03:50 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> - Patch adds MaxSpecies support for all types of vectors.
>> - New factory methods and VectorPayload classes for various kinds of vector, shuffle and mask payloads.
>> - Disable CDS archiving of VectorPayload classes.
>> - Summary of high level flow :-
>>   1/ Max species payloads encapsulate @multifield annotated field accepting -1 value as bundle size parameter.
>>   2/ For Vector payload bundle size is determined using maximum vector size supported by the target. 
>>   3/ For Shuffles and Masks payloads multifield bundle size is a function of maximum vector size and vector lane size.
>>   4/ Based on the dynamic bundle size parser creates a separate FieldInfo structure for each base and synthetic multifield and rest of the flow remains the same. 
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comments resolutions.

src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 663:

> 661: int VM_Version::max_vector_size(BasicType bt) {
> 662:   return MaxVectorSize / type2aelembytes(bt);
> 663: }

Please change to use the code in https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/cpu/aarch64/aarch64.ad#L2344 like what you have changed in x86 systems, together with the `Matcher::max_vector_size()`. Thanks a lot!

src/hotspot/cpu/x86/vm_version_x86.cpp line 3257:

> 3255:   default:
> 3256:     assert(false, "Unexpected basic type");
> 3257:   }

If the return value is 0, what is the actual number of the multifields in payload? It seems not reasonable for `0`. WDYT?

src/hotspot/share/oops/inlineKlass.cpp line 158:

> 156:   if (VectorSupport::is_vector_payload_mf(this) || VectorSupport::is_vector(this)) {
> 157:     return false;
> 158:   }

Why do you close the array flatten for vectors?

src/hotspot/share/prims/vectorSupport.cpp line 465:

> 463:   assert(payload_name, "");
> 464:   vmSymbolID sid = vmSymbols::find_sid(payload_name);
> 465:   int size = VM_Version::max_vector_size(T_BYTE);

It seems we'd better check whether the return value `size` is `0`. If so, a max with 64-bit size is needed like what we have done in java code. See https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorShape.java#L267

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/AbstractMask.java line 143:

> 141:                 species.maskType(), species.elementType(), vspecies().laneCount,
> 142:                 this, species,
> 143:                 (m, s) -> VectorMask.fromLong(s, m.toLong()).check(s));

Any issues with the original `maskFactory()` ?

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/931#discussion_r1360016602
PR Review Comment: https://git.openjdk.org/valhalla/pull/931#discussion_r1360010718
PR Review Comment: https://git.openjdk.org/valhalla/pull/931#discussion_r1360001591
PR Review Comment: https://git.openjdk.org/valhalla/pull/931#discussion_r1360015893
PR Review Comment: https://git.openjdk.org/valhalla/pull/931#discussion_r1360002100



More information about the valhalla-dev mailing list