On Thu, 25 Sep 2025 10:47:39 GMT, Christian Hagedorn <[email protected]> wrote:
> This patch fixes various issues in C1 and C2 (11 bugs in total) when running > with `-XX:+UseCompactObjectHeaders`. > > Most of the bugs could either be traced back to: > - Unconditionally assuming old layout with klass pointer following > immediately the mark word. > - Small mistakes when merging JEP 450. > - Using the wrong basic type `T_OBJECT` for flat arrays instead of > `T_FLAT_ELEMENT`. In Valhalla, we require that the elements of an array are > aligned:https://github.com/openjdk/valhalla/blob/04efe5c66b70b9dca1a5c538c9a04e7ad93c107a/src/hotspot/share/oops/arrayOop.hpp#L94-L97 > https://github.com/openjdk/valhalla/blob/04efe5c66b70b9dca1a5c538c9a04e7ad93c107a/src/hotspot/share/oops/arrayOop.hpp#L57-L61 > When we now wrongly use `T_OBJECT`, it is not a problem with default flags > (using compressed klass pointers and no COH): An array has a 12 byte header + > 4 byte length which is 16 bytes in total - no alignment required. But with > COH, we now have a header of 8 bytes + 4 byte length. We now require > alignment but when we pass in `T_OBJECT`, we miss the alignment which leads > to crashes down the line. I went through all the uses of > `base_offset_in_bytes()` and fixed more such issues. > > To review: I made separate commits for each fixed bug, each message > summarizing the found issue. To better understand individual changes of the > patch, it might help to look at the commits separately. > > **Testing:** > - Apart from one bug, I could trigger them with existing tests. So, I have > not added new tests for these issues separately. > - One bug only manifested with a new test (`test127a`) which I added with > this patch. > - tier1-5 + compiler stress: > - No additional flags, i.e. (mostly) without COH: Looks good and only known > failures > - With explicitly setting COH: > - A lot of CDS related failures on which @matias9927 is working > ([JDK-8367959](https://bugs.openjdk.org/browse/JDK-8367959)) > - Test timeout with `TestIntrinsics`: Unrelated to COH but only > triggering with COH > ([JDK-8368274](https://bugs.openjdk.org/browse/JDK-8368274)). > - Otherwise, looks good with only known failures. > > Thanks, > Christian This pull request has now been integrated. Changeset: df4b1cf3 Author: Christian Hagedorn <[email protected]> URL: https://git.openjdk.org/valhalla/commit/df4b1cf335e238ba2e54de9065056bc97df0d7a8 Stats: 157 lines in 12 files changed: 93 ins; 37 del; 27 mod 8348972: [lworld] C1/C2: Update Valhalla with UseCompactObjectHeaders for oop->klass load/stores ------------- PR: https://git.openjdk.org/valhalla/pull/1632
