On Mon, 19 Apr 2021 19:00:09 GMT, Vladimir Kozlov <k...@openjdk.org> wrote:

>> 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
>
> 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.

> 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`.

So comment as for `DECLARE_INT_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`?

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.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3558

Reply via email to