[lworld] RFR: 8364191: [lworld] Accesses to atomic flat fields prevent scalar replacement [v2]
Tobias Hartmann
thartmann at openjdk.org
Sat Oct 18 19:31:22 UTC 2025
On Sat, 4 Oct 2025 11:52:16 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:
>> Hi @merykitty Sorry for the delay in reviewing this, I was busy with the the [array metadata refactoring](https://github.com/openjdk/valhalla/pull/1452) and hunting down nasty AArch64 crashes ([JDK-8364579](https://bugs.openjdk.org/browse/JDK-8364579) / [JDK-8365996](https://bugs.openjdk.org/browse/JDK-8365996)). This is great work! I see that you converted the PR back to draft, is it still ready to review?
>
> @TobiHartmann Hi Tobias, I am having difficulties understanding how our current flat accesses deal with GC barriers. IIUC, every time an access is made to a memory region that may contain an oop, we need a GC barrier (which itself may be a nop). However, looking at our current implementation, I only see that being handled for G1GC with `StoreLSpecialNode`. For Serial, Parralel, and Shenandoah, the access is made as if the memory is a long, which I assume would not trigger any GC barrier. When I try to add an assertion that a flat access that contains an oop cannot be made for anything other than G1GC, the assert fires.
@merykitty Hi Quan-Anh, sorry for the delay, I missed this message.
> I only see that being handled for G1GC with StoreLSpecialNode
G1 is handled specially because of late barrier expansion:
https://github.com/openjdk/valhalla/blob/c1774b0b6b129623f77fbb7096d332565f148677/src/hotspot/share/opto/inlinetypenode.cpp#L660-L693
For the other GCs, we emit the barriers already during parsing (the information is propagated via `C2ParseAccess`):
https://github.com/openjdk/valhalla/blob/c1774b0b6b129623f77fbb7096d332565f148677/src/hotspot/share/gc/shared/c1/modRefBarrierSetC1.cpp#L40-L53
and
https://github.com/openjdk/valhalla/blob/c1774b0b6b129623f77fbb7096d332565f148677/src/hotspot/share/gc/shared/c2/modRefBarrierSetC2.cpp#L59-L70
I added this with https://github.com/openjdk/valhalla/pull/1318 and as mentioned in the PR, this is a bit of a hacky first implementation. We should refactor / improve this later (tracked by [JDK-8350865](https://bugs.openjdk.org/browse/JDK-8350865)).
Does that help?
If you get a chance, could you merge this PR with latest `lworld`? We see some issues that look similar to this and would like to verify if your patch helps. Thanks!
-------------
PR Comment: https://git.openjdk.org/valhalla/pull/1518#issuecomment-3405862656
More information about the valhalla-dev
mailing list