[lworld] RFR: 8348972: [lworld] C1/C2: Update Valhalla with UseCompactObjectHeaders for oop->klass load/stores

Christian Hagedorn chagedorn at openjdk.org
Mon Oct 6 08:23:16 UTC 2025


On Thu, 25 Sep 2025 10:47:39 GMT, Christian Hagedorn <chagedorn at openjdk.org> 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

Thanks Tobias!

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

PR Comment: https://git.openjdk.org/valhalla/pull/1632#issuecomment-3370444207


More information about the valhalla-dev mailing list