On Wed, 23 Aug 2023 22:40:23 GMT, David Holmes <dhol...@openjdk.org> wrote:

>> This change wasn't in the original patch, but the 
>> `GENERATE_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY` change above it was.  I found 
>> it when I noticed the use of `GENERATE_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY` 
>> and thought it should be renamed too. I was also doing "static volatile" -> 
>> "volatile stack" at the time so that is also part of this macros rename. 
>> 
>> As David points out, the purpose is to make it more general purpose, 
>> although for this macro only the name change is needed 
>> (`GENERATE_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY also has a minor 
>> implementation change that was needed).
>> 
>> The comments for both of these macros references "static pointer volatile" 
>> entries. That should changed to "volatile static". The type no longer needs 
>> to be a pointer type. 
>> 
>> I'm not certain what this CHECK macro does, but it was pre-existing, and 
>> there is also the `CHECK_STATIC_VM_STRUCT_ENTRY` macro above it which is 
>> identical except of the absence of "volatile" in the name and 
>> implementation. I think it verifies that the type  specified in the 
>> vmstructs definition of the field is actually the same as the field itself.  
>> The macro will generate a compiler error if a cast was needed. I think this 
>> type of check can only be done with static fields. You would need an 
>> instance of the class/struct to do a similar check on an instance field. 
>> Thus CHECK_NONSTATIC_VM_STRUCT_ENTRY is much more complex and seems to be a 
>> runtime check that will assert if it fails.
>
> I can't convince myself that `volatile` is in the right place in `type 
> volatile * dummy`. But `volatile * dummy` means that the pointer is volatile 
> as opposed to the type being pointed to, which would match with the original 
> macro name's use of `PTR_VOLATILE`.

@dholmes-ora I'm not sure what is needed to move forward with this. Are you OK 
with the change?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15373#discussion_r1306091948

Reply via email to