On Mon, 19 Apr 2021 09:46:17 GMT, Doug Simon <dnsi...@openjdk.org> wrote:
>> While porting >> [JDK-8224974](https://bugs.openjdk.java.net/browse/JDK-8224974) to Graal, I >> noticed that new CPU features were defined for x86 and AArch64 without being >> exposed via JVMCI. To avoid this problem in future, this PR updates x86 and >> AArch64 to define CPU features with a single macro that is used to generate >> enum declarations as well as vmstructs entries. >> >> In addition, the JVMCI API is updated to exposes the new CPU feature >> constants and now has a check that ensure these constants are in sync with >> the underlying macro definition. > > Doug Simon has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > avoid use of a lambda in JVMCI initialization Please, update copyright years in files you touched. src/hotspot/cpu/aarch64/vm_version_aarch64.hpp line 118: > 116: decl(STXR_PREFETCH, "stxr_prefetch", 29) \ > 117: decl(A53MAC, "a53mac", 30) > 118: #define DECLARE_CPU_FEATURE_FLAG(id, name, bit) CPU_##id = (1 << bit), Add empty line before to separate `CPU_FEATURE_FLAGS` macro. 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. src/hotspot/cpu/x86/vm_version_x86.hpp line 363: > 361: decl(AVX512_VBMI, "avx512_vbmi", 45) /* Vector BMI > instructions */ \ > 362: decl(HV, "hv", 46) /* Hypervisor > instructions */ > 363: #define DECLARE_CPU_FEATURE_FLAG(id, name, bit) CPU_##id = (1ULL << bit), Add empty line before it to separate `CPU_FEATURE_FLAGS` macro more clear. 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`. src/hotspot/share/jvmci/vmStructs_jvmci.cpp line 769: > 767: > 768: #define DECLARE_LONG_CPU_FEATURE_CONSTANT(id, name, bit) > GENERATE_VM_LONG_CONSTANT_ENTRY(VM_Version::CPU_##id) > 769: #define VM_LONG_CPU_FEATURE_CONSTANTS > CPU_FEATURE_FLAGS(DECLARE_LONG_CPU_FEATURE_CONSTANT) Missing `#undef DECLARE_LONG_CPU_FEATURE_CONSTANT`. 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`? ------------- PR: https://git.openjdk.java.net/jdk/pull/3558
