[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