RFR: 8368811: [Leyden] Use AOTRuntimeConstants table for card_table::_byte_map_base [v2]
Andrew Dinn
adinn at openjdk.org
Thu Oct 2 08:53:25 UTC 2025
On Thu, 2 Oct 2025 05:30:02 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:
>> Currently we use `Relocation` info to patch `card_table::_byte_map_base` referenced in compiled code. This is not completely correct since this is not address. We currently have special check in `AOTCodeCache::init2()` to skip AOT code generation and usage if `byte_map_base` is not relocatable [aotCodeCache.cpp#L341](https://github.com/openjdk/leyden/blob/premain/src/hotspot/share/code/aotCodeCache.cpp#L341)
>>
>> To avoid this we should use existing `AOTRuntimeConstants` table to load `byte_map_base` from it.
>>
>> I also added few missing loads `card_shift` from `AOTRuntimeConstants` table. Actually I am not sure why we have it in `AOTRuntimeConstants` table. From what I see, it is based on `GCCardSizeInBytes` flag [cardTable.cpp#L46](https://github.com/openjdk/leyden/blob/premain/src/hotspot/share/gc/shared/cardTable.cpp#L46) and never modified. The flags is not adjusted ergonomically. So we can simple record the flag in AOT code configuration and verify it when loading AOT code.
>>
>> @adinn what do you think about `card_shift` in `AOTRuntimeConstants` table? You added it there.
>>
>> Changes were testing tier1-5.
>
> Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision:
>
> Removed card_shift from AOT constants table. Fixed minimal VM build.
Looks ok, modulo a couple of optional suggested tweaks and a name correction for the arm failure.
I'm not sure why we are seeing a Shenandoah timeout but it ought to be unrelated as Shenandoah barriers are 1) independent of the (G1) region grain size and 2) access the card table base via a thread local.
src/hotspot/cpu/aarch64/gc/shared/cardTableBarrierSetAssembler_aarch64.cpp line 48:
> 46: if (UseCondCardMark) {
> 47: Label L_already_dirty;
> 48: __ ldrb(rscratch2, Address(obj, rscratch));
Do we need an extra precondition?
`precond(!UseCondCardMark || rscratch != rscratch2)`
src/hotspot/share/gc/shared/c1/cardTableBarrierSetC1.cpp line 68:
> 66:
> 67: #ifdef CARDTABLEBARRIERSET_POST_BARRIER_HELPER
> 68: assert(!aotCodeCache::is_on(), "this path is not implemented");
Suggestion:
assert(!AOTCodeCache::is_on(), "this path is not implemented");
src/hotspot/share/gc/shared/c1/cardTableBarrierSetC1.cpp line 88:
> 86: __ move(byte_map_base_adr, byte_map_base_reg);
> 87: LIR_Address* byte_map_base_indirect = new LIR_Address(byte_map_base_reg, 0, T_LONG);
> 88: //LIR_Opr byte_map_base = gen->new_pointer_register();
Do we need this comment?
-------------
Marked as reviewed by adinn (Committer).
PR Review: https://git.openjdk.org/leyden/pull/102#pullrequestreview-3293207725
PR Review Comment: https://git.openjdk.org/leyden/pull/102#discussion_r2397670560
PR Review Comment: https://git.openjdk.org/leyden/pull/102#discussion_r2397781476
PR Review Comment: https://git.openjdk.org/leyden/pull/102#discussion_r2397735318
More information about the leyden-dev
mailing list