RFR: 8265403: consolidate definition of CPU features [v3]

[Vladimir Kozlov]

On Mon, 19 Apr 2021 19:38:52 GMT, Doug Simon <dnsimon at 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