On Wed, 23 Aug 2023 07:23:22 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> src/hotspot/share/runtime/vmStructs.hpp line 211:
>>
>>> 209: // e.g.: "static ObjectMonitor * volatile g_block_list;"
>>> 210: #define CHECK_VOLATILE_STATIC_VM_STRUCT_ENTRY(typeName, fieldName,
>>> type) \
>>> 211: {type volatile * dummy = &typeName::fieldName; }
>>
>> It is not clear why the `PTR_` suffix is removed from the name.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15373#discussion_r1303375622
