RFR: 8363837: Move StubRoutines::_crc_table_adr initialization to preuniverse stubs

Andrew Dinn adinn at openjdk.org
Wed Jul 23 13:21:54 UTC 2025


On Wed, 23 Jul 2025 01:07:58 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> `StubRoutines::_crc_table_adr` and `_crc32c_table_addr` are used by initial stubs. In Leyden these addresses should be recorded in `AOTCodeAddressTable` to be used by AOTed stubs. But recording in `AOTCodeAddressTable` is done in `AOTCodeCache::init2()` which is called before initial stubs are generated and `_crc*_table_addr` are set.
>> 
>> We need to move `_crc_table_addr` and `_crc32c_table_addr` initialization to `preuniverse` blob to be available in `AOTCodeCache::init2()`.
>> 
>> I also renamed `_crc_table_adr` to `_crc_table_addr` to match other names.
>> 
>> During testing I found that `-Xlog:stubs` affects empty blobs generation.  There was typo there: `return nullptr;` was in wrong place.
>> 
>> I have to specify sizes of `preuniverse` blobs so they are called after I fixed typo.  `32` bytes is cache line size on most CPUs. Note, there will be no assembler code generation for this case: `_crc*_table_addr` are initialized by address of C tables. 
>> 
>> I did not move `_crc*_table_addr` declaration in `stubDeclarations.hpp` from `init` to `preuniverse` blob. I tried but there is issue: they require to specify stub name and I can move `updateBytesCRC32` stub declaration.  I tries add fake stub but it did not work.  I would prefer if I could use `nullptr` instead of stub in such case. But it will complicate other code. I think it is fine `do_entry()` macro only creates field and accessor function.
>> 
>> Tested: tier1-4,tier10-rt,xcomp,stress
>
> @TheRealMDoerr @offamitkumar @RealFYang please test these changes on your platforms.

@vnkozlov I'll note straight off that the situation here is complicated by the fact that `crc_table_adr` and `crc32c_table_addr` are pseudo-'entries'. They do not actually hold (code or data) addresses in generated code. What they really identify is an arch-specific address for foreign (C++) data that is assigned as a side-effect of performing generation for other entries. the only reason for marking them as entries is so we get a generated field and associated getter in class `StubRoutines`.

I'm not sure we need to care too much about their pseudo-entry status. More importantly, I dislike your solution of moving the initialization into `generate_preuniverse_stubs()`. This moves that initialization step away from the generator code that uses the resulting value, making it harder to see how the intrinsic works. It also has the unfortunate side effect of forcing us to create a preuinverse blob for every arch even if when we don't really need one.

You are right that we cannot add these addresses to the external addresses list under `AOTCodeCache::init2()` (via the call to `table->init_extrs()`) because the assignment of the pseudo-entry has not yet happened. However, we have two alternatives.

1) We can simply pretend that they are real entries add them to the stub addresses list under `AOTCodeCache::init_early_stubs_table()` (via the call to `table->init_early_stubs()`). That will successfully advertise them  after they have been created as addresses that need relocation at AOT Cache load. However, it will also misclassify them as addresses in generated code when they are actually foreign addresses. Do we care about that misclassification (do we use a different reloc for stubs addresses vs external addresses?)

2) We can leave the external addresses table open at the end of the call to table->init_extrs()`. When we enter table->init_early_stubs() we can add these two extra 'pseudo-stub' addresses to the external addresses list and close it before registering the early stubs addresses.

Both of these seem to me to be simpler and cleaner ways to deal with the address publication problem. What do you think?

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

PR Comment: https://git.openjdk.org/jdk/pull/26434#issuecomment-3108328211


More information about the graal-dev mailing list