[lworld] RFR: 8366717: [lworld] Cleanup defensive fixing of JDK-8365996 [v2]
Marc Chevalier
mchevalier at openjdk.org
Fri Dec 19 08:44:08 UTC 2025
> Let's proceed piece by piece.
>
> # `G1BarrierSetAssembler::g1_write_barrier_pre` in `g1BarrierSetAssembler_aarch64.cpp`
>
> https://github.com/openjdk/valhalla/blob/1077e4f9f06336af30d95fc28cbab4d31c9f2a44/src/hotspot/cpu/aarch64/gc/g1/g1BarrierSetAssembler_aarch64.cpp#L216-L220
>
> `push_call_clobbered_registers`/`pop_call_clobbered_registers` should be enough around a runtime call, that's what clobbered registers are.
>
> But let's dig a bit, that will be useful later!
>
>
> push_call_clobbered_registers()
> => push_call_clobbered_registers_except(exclude = empty set)
> => push(call_clobbered_gp_registers() - exclude, sp) // with exclude = empty set
>
> So, we save at least `call_clobbered_gp_registers` which is defined as:
>
> https://github.com/openjdk/valhalla/blob/1077e4f9f06336af30d95fc28cbab4d31c9f2a44/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L3614-L3620
>
> So, we save r0-r7 and r10-r17
>
> # `CardTableBarrierSetAssembler::oop_store_at` in `cardTableBarrierSetAssembler_aarch64.cpp`
>
> https://github.com/openjdk/valhalla/blob/1077e4f9f06336af30d95fc28cbab4d31c9f2a44/src/hotspot/cpu/aarch64/gc/shared/cardTableBarrierSetAssembler_aarch64.cpp#L116-L124
>
> Digging the history looks like it still makes sense, it doesn't look like an accident. If we remove the if-branch, tests are totally on fire. Let's keep it.
>
> # `CardTableBarrierSetAssembler::oop_store_at` in `cardTableBarrierSetAssembler_x86.cpp`
>
> Same as before. But it's not even guarantee that `InlineTypePassFieldsAsArgs` is true. We can have such a backtrace:
>
> CardTableBarrierSetAssembler::oop_store_at (cardTableBarrierSetAssembler_x86.cpp:184)
> CardTableBarrierSetAssembler::store_at (cardTableBarrierSetAssembler_x86.cpp:90)
> MacroAssembler::store_heap_oop (macroAssembler_x86.cpp:5515)
> do_oop_store (templateTable_x86.cpp:148)
> (called, for instance, from TemplateTable::putfield_or_static_helper (templateTable_x86.cpp:2964))
>
> where the last give `r8` for `tmp3`. It is not quite clear to me why we don't have a problem in mainline, because it looks like corrupting address base register is a bad idea given that we use it after.
>
> # `MacroAssembler::unpack_inline_helper` in `macroAssembler_aarch64.cpp`
>
> ## First part
> https://github.com/openjdk/valhalla/blob/1077e4f9f06336af30d95fc28cbab4d31c9f2a44/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L7163-L7166
>
> Yes, these registers are saved, as we saw above! I've added some asserts to make sure that they are still in the `call_clobbered_gp_registers()` set. But it seems a bit ...
Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
Pick the right comment again
-------------
Changes:
- all: https://git.openjdk.org/valhalla/pull/1824/files
- new: https://git.openjdk.org/valhalla/pull/1824/files/3f013c81..bf65d118
Webrevs:
- full: https://webrevs.openjdk.org/?repo=valhalla&pr=1824&range=01
- incr: https://webrevs.openjdk.org/?repo=valhalla&pr=1824&range=00-01
Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
Patch: https://git.openjdk.org/valhalla/pull/1824.diff
Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1824/head:pull/1824
PR: https://git.openjdk.org/valhalla/pull/1824
More information about the valhalla-dev
mailing list