On Mon, 26 Jan 2026 10:23:01 GMT, Axel Boldt-Christmas <[email protected]>
wrote:
>> Hello,
>>
>> Please refer to the JBS issue for a more detailed description of the
>> background of this change. In summary, I suggest we only keep the array of
>> InlineLayoutInfo for InstanceKlasses which need it, which are Klasses that
>> have fields that have been inlined.
>>
>> To make the transition to this easier, I suggest we change the following
>> properties in FieldLayoutBuilder:
>>
>> _has_inline_type_fields
>> _has_flattening_information
>>
>> to
>>
>> _has_inlineable_fields
>> _has_inlined_fields
>>
>> The `_has_inlineable_fields` property is only used for printing and
>> `_has_inlined_fields` is the property we expose out to the ClassFileParser,
>> telling us that this class has inlined fields, so the array of
>> InlineLayoutInfo must be "preserved" and is possible to read from. Hence,
>> the array is now only safe to access if `InstanceKlass::has_inlined_fields`
>> is true, or simply if the actual field being accessed is flat
>> (`fieldDescriptor::is_flat`).
>>
>> I only found one place (in ciReplay.cpp) where we access the array of
>> InlineLayoutInfo even though we might not have any inlined fields and only
>> fields that are inlineable. I've changed this to use the normal "reference"
>> path for fields that aren't flat.
>>
>> Testing:
>> * Oracle's tier1-5, hotspot_valhalla and jdk_valhalla
>
> src/hotspot/share/classfile/classFileParser.cpp line 6393:
>
>> 6391: }
>> 6392:
>> 6393: void ClassFileParser::set_inline_layout_info_klass(int field_index,
>> Klass* klass, TRAPS) {
>
> This should be typed as `InstanceKlass* klass`.
>
> 2 out 3 call sites have that type, and the remaining can be trivially changed
> to `InstanceKlass*`
> https://github.com/jsikstro/valhalla/blob/ae50201a1b19eaa7a53bce222dfc93ab61dd31f9/src/hotspot/share/classfile/classFileParser.cpp#L6293
Good catch, I agree.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1966#discussion_r2727314938
