[lworld+vector] RFR: 8320815: [lworld+vector] Fix vector API jtreg crashes with "-XX:InlineFieldMaxFlatSize=0"

[Xiaohong Gong]

On Fri, 8 Dec 2023 09:00:52 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Current Vector API jtreg crashes with `-XX:InlineFieldMaxFlatSize=0` with several kinds of issues. This option makes all the value object fields can not be flattened.
>> 
>> This patch fixes them with following main changes:
>> 
>>  1) Remove the fields count check for `Vector/VectorPayload` objects when scalaring the value objects across safepoints. The check on vector objects fails when its payload field is not flattened (see [1]). The check is not right and can be removed.
>> 
>>  2) Insert two additional inputs of `InlineTypeNode` to safepoint when creating the `SafePointScalarObjectNode` for a value object. This is missing when compiler handles an allocation merge with inline type. It causes the assertion [2] fails.
>> 
>>  3) Forbit optimizing a `LoadN` to `EncodeP (InlineType)`, if the `InlineType` is the payload field of `VectorBox`. This fixes the
>> assertion failure [3].
>> 
>> When loading from an `InlineType`, C2 compiler tries to look for its field value with the matching offset. If the matched field is a
>> flattened value object, its field with the matching offset is searched recursively. Otherwise, the field itself is used. After the field
>> value is found, the load can be saved, and the field value is used as the replacement. If it is an object loading (i.e. `LoadN`), the
>> matched field value is expected to be a valid oop.
>> 
>> The issue happens when the address base is `VectorBox` and the matched field is a non-flattened `InlineType`. Since `VectorBox` is expanded lately, its memory and the memory for the field are not allocated at this moment. Optimizing `LoadN` to `EncodeP` with the un-allocated `InlineType` makes it possible to the miss the buffer.
>> 
>>  4) Explicitly check whether the payload field is flattened when expanding `VectorBox`. Allocate the buffer and store the vector value to the memory if the payload is not flattened.
>> 
>> The similar operations are handled in `InlineTypeNode::store()` before. But the graph is more complex and not right for some vector API cases. We hit assertion [4].
>> 
>> Tests: All tests pass with `-XX:InlineFieldMaxFlatSize=0` now. And no new regressions are found.
>> 
>> [1] https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/opto/inlinetypenode.cpp#L318
>> [2] https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/opto/output.cpp#L812
>> [3] https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/opto/compile.cpp#L2050
>> [4] https://github.com/openjdk...
>
> src/hotspot/share/opto/inlinetypenode.cpp line 319:
> 
>> 317:   } else if (is_vector_payload(vk)) {
>> 318:      assert(field_value(0)->as_InlineType()->field_count() == nfields, "");
>> 319:   }
> 
> Its not very clear, why will these assertions offend because C2 IR is in synchronism with ci Model. Also given that C2 always creates a InlineTypeNode for a value object. Even for a non-flattened field compile injects a re-materializing InlineTypeNode from the loaded value oop.

The assertion fails on line 318. Use `Int256Vector` as an example, if the payload is not flattened and the multifields are not vectorized, `field_value(0)->as_InlineType()->field_count()` should be 8, while the `nfields` of the vector value object is 1.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/958#discussion_r1421876773