On Mon, 19 Apr 2021 19:38:52 GMT, Doug Simon <dnsi...@openjdk.org> wrote:
>> src/hotspot/cpu/x86/vmStructs_x86.hpp line 45:
>>
>>> 43:
>>> 44: #define DECLARE_LONG_CPU_FEATURE_CONSTANT(id, name, bit)
>>> GENERATE_VM_LONG_CONSTANT_ENTRY(VM_Version::CPU_##id)
>>> 45: #define VM_LONG_CPU_FEATURE_CONSTANTS
>>> CPU_FEATURE_FLAGS(DECLARE_LONG_CPU_FEATURE_CONSTANT)
>>
>> Do we need to keep `VM_LONG_CONSTANTS_CPU` after you removed its body here
>> and in vmStructs_jvmci.cpp?
>>
>> What about `VM_INT_CONSTANTS_CPU` here? vmStructs_jvmci.cpp duplicates it.
>
> `vmStructs.cpp` and `vmStructs_jvmci.cpp` are disjoint. This file (i.e.
> `vmStructs_x86.hpp`) is only used by `vmStructs.cpp`.
> `vmStructs.cpp` expects all macros such as `VM_LONG_CONSTANTS_CPU` to be
> defined.
> `vmStructs_jvmci.cpp` will provide a dummy definition for missing macros.
Got it. Even so they are empty everywhere :(
>> src/hotspot/share/jvmci/vmStructs_jvmci.cpp line 753:
>>
>>> 751:
>>> 752: #define DECLARE_INT_CPU_FEATURE_CONSTANT(id, name, bit)
>>> GENERATE_VM_INT_CONSTANT_ENTRY(VM_Version::CPU_##id)
>>> 753: #define VM_INT_CPU_FEATURE_CONSTANTS
>>> CPU_FEATURE_FLAGS(DECLARE_INT_CPU_FEATURE_CONSTANT)
>>
>> Missing `#undef DECLARE_INT_CPU_FEATURE_CONSTANT`.
>
> No, it must stay defined up to the point `VM_INT_CPU_FEATURE_CONSTANTS` is
> used. Since this is a `.cpp` file, it's ok to leave it defined.
I see.
>> src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotJVMCIBackendFactory.java
>> line 66:
>>
>>> 64: long bitMask = e.getValue();
>>> 65: String key = e.getKey();
>>> 66: if (key.startsWith("VM_Version::CPU_")) {
>>
>> As I understand this code, it goes over `constants` values passed from VM
>> and Trying to map them to `enumType`. It catches cases when a value is
>> missing in `enumType`. What about case when `enumType` has `extra` value
>> which is not defined in `constants`?
>
> We could warn about that but cannot remove it without breaking backwards
> capability for JVMCI wrt Graal. Such a deleted capability will simply be seen
> as "not supported" by Graal.
Okay.
-------------
PR: https://git.openjdk.java.net/jdk/pull/3558
