[lworld] RFR: 8376059: [lworld] Aarch64: StoreLSpecial with 1 oop does not save register in pre-barrier [v2]

[Tobias Hartmann]

On Mon, 2 Feb 2026 17:23:39 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Hi,
>> 
>> Many thanks to @stefank for finding the issue and helping investigating it. This PR fixes a couple of issues:
>> 
>> - `g1StoreLSpecialOneOop` does not save `mem` and `src` during the GC pre-barrier, this risks clobber these values, which may lead to crashes or memory corruption when doing the store instruction.
>> - It seems that the value passed to the post-barrier should be an uncompressed oop, this requires us to decode the narrow oop instead of simply zero-extending it.
>> - Relax the restriction on the `src` register to be an arbitrary register.
>> - Split the rule into 2 so that we can avoid cloberring an additional register when the offset of the oop inside the payload is 0.
>> 
>> Testing:
>> - [x] tier1, linux-aarch64, with `-XX:+StressGCM -XX:InitiatingHeapOccupancyPercent=0 -Xmn10m`
>> - [x] tier1-4, valhalla-comp-stress, linux-aarch64
>> - [x] tier1-4, valhalla-comp-stress, linux-aarch64, with `-XX:+StressGCM -XX:InitiatingHeapOccupancyPercent=0 -Xmn10m`
>> 
>> Please take a look and leave your reviews, thanks a lot.
>
> Quan Anh Mai has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Saving the register in the slow path instead
>  - recompute tmp4 instead of saving it

Looks good to me! I added a few minor comments.

I also took an Action Item to add `-XX:+StressGCM -XX:InitiatingHeapOccupancyPercent=0` and some more flags to our internal stress testing.

src/hotspot/cpu/aarch64/gc/g1/g1_aarch64.ad line 79:

> 77: // - Do no set/overwrite barrier data here, also handle G1C2BarrierPostNotNull
> 78: // - Move this into the .m4?
> 79: instruct g1StoreLSpecialOneOopOff0(indirect mem, iRegLNoSp src, immI0 off, iRegPNoSp tmp1, iRegPNoSp tmp2, iRegPNoSp tmp3, rFlagsReg cr)

Should we do the same on x64?

src/hotspot/cpu/aarch64/gc/g1/g1_aarch64.ad line 85:

> 83:   effect(TEMP tmp1, TEMP tmp2, TEMP tmp3, KILL cr);
> 84:   ins_cost(INSN_COST);
> 85:   format %{ "str  $src, $mem\t# g1StoreLSpecialOneOop" %}

The comment needs to be updated.

src/hotspot/cpu/aarch64/gc/g1/g1_aarch64.ad line 97:

> 95:     __ str($src$$Register, $mem$$Register);
> 96: 
> 97:     __ ubfm($tmp1$$Register, $src$$Register, 0, 31);

Please add a comment here.

src/hotspot/cpu/aarch64/gc/g1/g1_aarch64.ad line 119:

> 117: 
> 118:     // Adjust address to point to narrow oop
> 119:     __ add($tmp4$$Register, $mem$$Register, 4);

I think we should have asserts on `oop_off_1` and `oop_off_2` in `StoreFlatNode::expand_atomic` that check that `off` is either zero or four. Otherwise we would hit a "bad ad file" failure during matching. Also, a comment explaining that this is due to alignment constraints would be nice.

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

Marked as reviewed by thartmann (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/2013#pullrequestreview-3744076946
PR Review Comment: https://git.openjdk.org/valhalla/pull/2013#discussion_r2758083421
PR Review Comment: https://git.openjdk.org/valhalla/pull/2013#discussion_r2758085788
PR Review Comment: https://git.openjdk.org/valhalla/pull/2013#discussion_r2758209368
PR Review Comment: https://git.openjdk.org/valhalla/pull/2013#discussion_r2758150042