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

Xiaohong Gong xgong at openjdk.org
Mon Oct 16 07:43:10 UTC 2023


On Mon, 16 Oct 2023 06:21:56 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> 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?
>
> Array of vector may be heterogeneous in nature where each element points to a vector of different shape, thus array flatten do not make much sense to vectors. In additional during type resolution flat_array flag is set for imbalanced Phi (having different type of inputs e.g. VectorPayload64 and VectorPayload128) even though Phi's type is lower common ancestor type of participating inputs i.e., VectorPayload which itself is not an inlinetype, this leads to assertion failures in downstream flow.
> 
> https://github.com/openjdk/valhalla/blob/lworld/src/hotspot/share/opto/type.cpp#L4636
> 
> Instead of making a wider change in type resolution, where we flat_array setting also take into account exact type of PhiNode, I prefer to restrict array flatten for vectors.

Thanks for the explanation! Seems it also causes the jvm crash in `VectorXXXConversionTests`, but I didn't figure out the root cause yet.  Is it possible supporting the flat array for vector objects with some fixes in future? We may lose the performance with this change, right?

>> 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()` ?
>
> We cannot typecast value object with another value type, thus directly passing payload (m.getBits()) to maskFactory will cause compilation failures.

Sounds reasonable! But I'm curious why we didn't meet this issue before?

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/931#discussion_r1360232286
PR Review Comment: https://git.openjdk.org/valhalla/pull/931#discussion_r1360222221



More information about the valhalla-dev mailing list