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

[Roland Westrelin]

On Wed, 2 Apr 2025 14:11:34 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

>> 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()`?
>
> I think it should be
> 
> TypeAryKlassPtr::BOTTOM->maybe_java_subtype_of(tkls)
> 
> rather than
> 
> tkls->maybe_java_subtype_of(TypeAryKlassPtr::BOTTOM)
> 
> 
> My reasoning: if `TypeAryKlassPtr::BOTTOM` is `java.lang.Object + Cloneable + Serializable` any array is a subtype of that. But so is any class implementing these interfaces. As well as as any `Object` implementing more interfaces. But for these two last cases, we know they cannot be array, which is what we want to know: are we sure it's not an array, or could it be an array?
> 
> But if we check if `tkls` is a supertype of `java.lang.Object + Cloneable + Serializable`, then it has to be an `Object` (the most general class) and it implements a subset of `Cloneable` and `Serializable`. In this case, it can be an array. If `tkls` is not a super-type of `java.lang.Object + Cloneable + Serializable`, there are 2 cases:
> - either it is an array type directly (so, I think, in a way or another, we need to check for `is_instklassptr`), and so a fortiori it can be an array type.
> -  it's an instance type and then cannot be an array since there is nothing between array types and `java.lang.Object + Cloneable + Serializable`. I.e. there is no type `T` that is not an array type, that is a super-type of at least one array type and that is not a super-type of `java.lang.Object + Cloneable + Serializable` (that is that is not `java.lang.Object` or that implements at least another interface).
> 
> In other words, our question is
> 
> \exists T: T is an array type /\ T <= tkls
> 
> (where `A <= B` means `A is a subtype of B`) which is equivalent to
> 
>     tkls >= (java.lang.Object + Cloneable + Serializable)
> / (tkls <= (java.lang.Object + Cloneable + Serializable) /\ tkls is an array type)
> 
> 
> We can spare the call to `is_instklassptr` by using a virtual method instead or probably other mechanisms, that's an implementation detail. But I think we need to distinguish cases: both `int[]` and `MyClass + Cloneable + Serializable + MyInterface` are sub-types of `java.lang.Object + Cloneable + Serializable` but for one, we can conclude it's definitely an array, and the other, it's definitely not. Without distinguishing cases, the only sound approximation would be to that that everything can be an array (both sub and super types of `java.lang.Object + Cloneable + Serializable`).
> 
> Does that makes sense? Did I get something wrong? is the `BOTTOM` not what you had in mind?

Yes, what I suggested doesn't work indeed.

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

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