RFR: 8339466: Enumerate shared stubs and define static fields and names via declarations [v2]

Andrew Dinn adinn at openjdk.org
Wed Sep 4 10:27:19 UTC 2024


On Tue, 3 Sep 2024 18:24:16 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Andrew Dinn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix errors in ppc generator
>
> src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 2571:
> 
>> 2569: SafepointBlob* SharedRuntime::generate_handler_blob(sharedStubId id, address call_ptr) {
>> 2570:   assert((id >= sharedStubId::polling_page_vectors_safepoint_handler_id ||
>> 2571:           id <= sharedStubId::polling_page_return_handler_id),
> 
> This and all other similar assert checks depends on the order of stubs ID.
> I think such checks should be in `sharedStubId` where the order is defined:
> `sharedStubId::is_polling_page_id(id)`

Yes, defining a (non-PRODUCT) validation method for this is a good idea. However, I cannot create it in sharedStubId as it is an enum class. I will put it into class SharedRuntime. I'll also create methods `is_handle_id()` and `is_throw_id()`.

I was unsure about relying on order/grouping when I wrote this. On reflection, I think it would be safer to make `is_polling_page_id()` explicitly enumerate the relevant cases. It is ok to rely on order here with SharedRuntime stubs. We are just using the declarations in stubDeclarations.hpp to define enum tags and static fields so we can define an order that groups related stubs without any further constraint. However, the current C1 stubs declaration order defines the order of generation of the stubs. Likewise, when it comes to systematizing the OptoRuntime stubs we will also want to use the declarations to drive the calls to `generate_xxx_blob()` and `generate_stub()`. So, in those cases the declaration order primarily needs to respect stub-stub dependencies.

> src/hotspot/share/runtime/stubDeclarations.hpp line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved.
> 
> You need only one 2024 year

Fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20832#discussion_r1743477126
PR Review Comment: https://git.openjdk.org/jdk/pull/20832#discussion_r1743480713


More information about the hotspot-dev mailing list