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

[Marc Chevalier]

On Wed, 26 Mar 2025 09:16:17 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

I'm not sure how to write such an IR test.

I'm looking at [TestArrayGuardWithInterfaces.java](https://github.com/openjdk/jdk/blob/3e9a7a4aed168422473c941ff5626d0d65aaadfa/test/hotspot/jtreg/compiler/intrinsics/TestArrayGuardWithInterfaces.java).

I see the graphs of `test1` before and after, and the new one is smaller. But the nodes used are pretty much the same, or they don't feel clearly linked to interface checking: there is `DecodeNKlass` or `AddP`, but it doesn't seem obvious without having the graph under the eyes that it actually checks something meaningful. There are also less `If` (2 instead of 3), but once again, the test seems brittle. I also see that There is no more `Return` only `Halt` since we can now prove the function cannot return normally.

But on the graph of `test2` ends with two `Halt`: traps everywhere, even if there are paths on which `test2` doesn't throw. So the lack of `Return` doesn't sound very robust.

Overall, not sure what a good test would be. I can write a test that would not pass before and pass now, but I'm not convinced they would reliably catch regression, and that they won't break for unrelated reasons.

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

PR Comment: https://git.openjdk.org/jdk/pull/24245#issuecomment-2760760131