RFR: 8348853: Fold layout helper check for objects implementing non-array interfaces [v2]

[Roland Westrelin]

On Mon, 31 Mar 2025 06:49:50 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

>> If `TypeInstKlassPtr` represents an array type, it has to be `java.lang.Object`. From contraposition, if it is not `java.lang.Object`, we can conclude it is not an array, and we can skip some array checks, for instance.
>> 
>> In this PR, we improve this deduction with an interface base reasoning: arrays implements only Cloneable and Serializable, so if a type implements anything else, it cannot be an array.
>> 
>> This change partially reverts the changes from [JDK-8348631](https://bugs.openjdk.org/browse/JDK-8348631) (#23331) (in `LibraryCallKit::generate_array_guard_common`) and the test still passes.
>> 
>> The way interfaces are check might be done differently. The current situation is a balance between visibility (not to leak too much things explicitly private), having not overly general methods for one use-case and avoiding too concrete (and brittle) interfaces.
>> 
>> Tested with tier1..3, hs-precheckin-comp and hs-comp-stress
>> 
>> Thanks,
>> Marc
>
> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
> 
>   not reinventing the wheel

src/hotspot/share/opto/memnode.cpp line 2214:

> 2212:     if (tkls->offset() == in_bytes(Klass::layout_helper_offset()) &&
> 2213:         tkls->isa_instklassptr() && // not directly typed as an array
> 2214:         !tkls->is_instklassptr()->might_be_an_array() // not the supertype of all T[] (java.lang.Object) or has an interface that is not Serializable or Cloneable

Could we do the same by using `TypeKlassPtr::maybe_java_subtype_of(TypeAryKlassPtr::BOTTOM)` and define a `TypeAryKlassPtr::BOTTOM` to be a static field for the `array_interfaces`?

AFAICT,   `TypeKlassPtr::maybe_java_subtype_of()` already covers that case so it would avoid some logic duplication. Also in the test above, maybe you could simplify the test a little but by removing `tkls->isa_instklassptr()`?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24245#discussion_r2020893305