[lworld] Integrated: 8357785: [lworld] TestResolvedJavaType fails due to unexpected getInstanceFields order

[Marc Chevalier]

On Thu, 17 Jul 2025 12:07:56 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

> It's all a matter of order. In short, mainline doesn't sort objects fields by offset, but just take them as they come from the stream. The order can be arbitrary, but in some places, it matters it's the same everywhere. Valhalla is sorting too much for mainline's taste, a merge caused clashes.
> 
> But maybe, a bit of archeology:
> - Mainline [JDK-8350892: [JVMCI] Align ResolvedJavaType.getInstanceFields with Class.getDeclaredFields](https://bugs.openjdk.org/browse/JDK-8350892): it removed some sorting of fields, keeping it consistent between `reassign_fields_by_klass` and `ciInstanceKlass::compute_nonstatic_fields()`: they both had fields offset-sorted, now they are both stream-sorted. Most of HotSpot agrees with this new order. This change also add a new test checking the order is consistent: `compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java`.
> 
> - Valhalla: [#1429](https://github.com/openjdk/valhalla/pull/1429) brings [JDK-8350892](https://bugs.openjdk.org/browse/JDK-8350892) in Valhalla. This merge is a bit mysterious to me,  how some sorting reappeared in Valhalla-untouched code. For instance, [JDK-8350892](https://bugs.openjdk.org/browse/JDK-8350892) removed the function `
> static int sort_field_by_offset(ciField** a, ciField** b)` and
>   ```cpp
>   // Now sort them by offset, 
>   // (In principle, they could mix with superclass fields.)
>   fields->sort(sort_field_by_offset);
>   ```
>   from `int ciInstanceKlass::compute_nonstatic_fields()` (in `ciInstanceKlass.cpp`), but the merge only removes the comment, but leave the `->sort()` and `sort_field_by_offset`. My guess is that it was a tentative fix for some failing tests after merge. Nevertheless `compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/VirtualObjectDebugInfoTest.java` is ProblemListed.
> 
> - Valhalla: After this merge, Valhalla continues to assume some ordering, as one can see in [JDK-8355076: [lworld] Incorrect declared_nonstatic_fields computation](https://bugs.openjdk.org/browse/JDK-8355076) that (among other things), makes a new occurrence of `->sort(sort_field_by_offset)`, to be consistent with `InlineKlass::collect_fields`. But now, everything is different! The new test introduced by [JDK-8350892](https://bugs.openjdk.org/browse/JDK-8350892) connects more things together, and enforce the same order in `ciInstanceKlass::compute_nonstatic_fields` and in `Class.getDeclaredFields` (reflexion related, I think. I don't know that so well). Moreover `ciInstance...

This pull request has now been integrated.

Changeset: b2e518b7
Author:    Marc Chevalier <mchevalier at openjdk.org>
Committer: Quan Anh Mai <qamai at openjdk.org>
URL:       https://git.openjdk.org/valhalla/commit/b2e518b7cf85527b90f34ba63bf5a1b775430cd0
Stats:     285 lines in 14 files changed: 147 ins; 75 del; 63 mod

8357785: [lworld] TestResolvedJavaType fails due to unexpected getInstanceFields order
8357186: [lworld] Clean up InlineKlass::collect_fields

Reviewed-by: qamai, thartmann

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

PR: https://git.openjdk.org/valhalla/pull/1511