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`. But in that case the use of the macro doesn't make much sense. Regardless of the field type, vmstructs doesn't deal with volatile pointers to fields. It deals with volatile fields (that can be pointers or scalar). I think if anything the macro has always been wrong and should contain `volatile type * dummy`. It probably never fails because it is taking a non-volatile type and assigning it to a volatile type, which always works if the types are otherwise the same. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15373#discussion_r1303630417
