[lworld] RFR: 8372113: [lworld] Fix various issues with TypeAryPtr and TypeAryKlassPtr [v6]
Quan Anh Mai
qamai at openjdk.org
Mon Dec 8 13:19:38 UTC 2025
On Mon, 8 Dec 2025 10:18:11 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:
>> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Should not clone MergeMem
>
> src/hotspot/share/opto/escape.cpp line 4630:
>
>> 4628: auto process_narrow_proj = [&](NarrowMemProjNode* proj) {
>> 4629: const TypePtr* adr_type = proj->adr_type();
>> 4630: const TypePtr* new_adr_type = tinst->with_offset(adr_type->offset());
>
> Should this use `TypeAryPtr::add_field_offset_and_offset`?
>
> Similar code also has a comment
>
> // In the case of a flat inline type array, each field has its
> // own slice so we need to extract the field being accessed from
> // the address computation
It is equivalent, but since this is about mirroring the offset part from the old slice (without instance id) to the new slice, I think it is more trivial what is happening using `with_offset`.
> src/hotspot/share/opto/macro.cpp line 548:
>
>> 546: }
>> 547: if (!init_value->is_InlineType()) {
>> 548: return nullptr;
>
> How can this happen?
`LibraryCallKit::inline_newArray` does not type check `init_val`, so if it is untyped (for example, the return value of `Class::new_instance`), then it will not be an `InlineTypeNode`.
> src/hotspot/share/opto/type.cpp line 679:
>
>> 677: TypeAryPtr::LONGS = TypeAryPtr::make(TypePtr::BotPTR, TypeAry::make(TypeLong::LONG ,TypeInt::POS, false, false, true, true, true), ciTypeArrayKlass::make(T_LONG), true, Offset::bottom);
>> 678: TypeAryPtr::FLOATS = TypeAryPtr::make(TypePtr::BotPTR, TypeAry::make(Type::FLOAT ,TypeInt::POS, false, false, true, true, true), ciTypeArrayKlass::make(T_FLOAT), true, Offset::bottom);
>> 679: TypeAryPtr::DOUBLES = TypeAryPtr::make(TypePtr::BotPTR, TypeAry::make(Type::DOUBLE ,TypeInt::POS, false, false, true, true, true), ciTypeArrayKlass::make(T_DOUBLE), true, Offset::bottom);
>
> Could we use default arguments here?
Yes, we can, but I feel that being explicit here would be better, as this combination will only make sense as the default for primitive arrays. What do you think?
-------------
PR Review Comment: https://git.openjdk.org/valhalla/pull/1755#discussion_r2598594733
PR Review Comment: https://git.openjdk.org/valhalla/pull/1755#discussion_r2598602175
PR Review Comment: https://git.openjdk.org/valhalla/pull/1755#discussion_r2598609202
More information about the valhalla-dev
mailing list