On Wed, 23 Aug 2023 18:09:24 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:

>> The intent was to generalize it but the comments also need changing, and now 
>> I'm not clear what this is actually doing.
>
> 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`.

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

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

Reply via email to