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