[lworld+vector] RFR: 8307715: Integrate VectorMask/Shuffle with value/primitive classes [v5]

[Jatin Bhateja]

On Fri, 19 May 2023 10:46:27 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> This patch changes the Vector API java side and hotspot compiler code to make the `VectorMask` and `VectorShuffle` operations work well after integrating with valhalla value/primitive classes.
>> 
>> Java side changes include:
>>  - Define the concrete `VectorMask/VectorShuffle `classes as value class.
>>  - Define the payload field type as primitive classes, which defines `MultiField` annotated field to save the element values.
>>  - Change all the relative API implementations, which use Unsafe APIs to access the new payload.
>>  - Move several VectorMask default operations to the abstract super class.
>>  - Minor code cleanup.
>> 
>> Compiler changes include:
>>  - Enable intrinsification for `VectorMask/VectorShuffle` related operations.
>>    Mask input/output of `VectorBox/VectorUnbox` is changed to the boolean vector format, to adapt C2's `InlineTypeNode`
>>    and new `VectorBox` features. Note that the mask input/output is a normal vector or predicate before.
>>  - Refine `VectorBox` expanding logic to make it right when the primitive payload instance is not flattened.
>>  - Minor code cleanup.
>> 
>> Basic VectorMask/VectorShuffle jtreg tests pass with"`-XX:+UnlockExperimentalVMOptions -XX:-EnableVectorSupport`",
>> and the basic unit tests work well after enabling vector support.
>
> Xiaohong Gong has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix jtreg failures

src/hotspot/share/opto/cfgnode.cpp line 2574:

> 2572:         }
> 2573:         const Type* t = phase->type(n);
> 2574:         if (n->is_InlineType() && !n->is_VectorBox() && (vk == NULL || vk == t->inline_klass())) {

We should do an upfront check for VectorBoxes to set can_optimize to false on line#2545 so that we never invoke _push_inline_types_through_ , it creates a new InlineTypeNode whose Oop fields may get stitched to VBAs, anyways we have a seperate routine for Phi handling _merge_through_phi_,

src/hotspot/share/opto/inlinetypenode.cpp line 448:

> 446:           Matcher::match_rule_supported_vector(VectorNode::replicate_opcode(bt), vec_len, bt)) {
> 447:           value = kit->gvn().transform(VectorNode::scalar2vector(value, vec_len, Type::get_const_type(field->type()), false));
> 448:         }

It may be needed for constant oops (gvn.zerocon(T_PRIMITIVE_OBJECT)) as created on following line.
Otherwise it may result into semantically incorrect IR which tries to load a vector from a constant.

src/hotspot/share/opto/library_call.cpp line 2323:

> 2321:       if (alloc != NULL) {
> 2322:         assert(alloc->_larval, "InlineType instance must be in _larval state for unsafe put operation.\n");
> 2323:       }

Larval bit check is needed to preserve the semantics of value objects which can be mutated only if its _larval bit is set,  allocation expansion explicitly generates IR to set this information in the mark word.  We should add a FIXME over if condition.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/845#discussion_r1203504921
PR Review Comment: https://git.openjdk.org/valhalla/pull/845#discussion_r1203545309
PR Review Comment: https://git.openjdk.org/valhalla/pull/845#discussion_r1203551961