RFR: 8318776: Require supports_cx8 to always be true [v5]

Daniel D. Daugherty dcubed at openjdk.org
Wed Nov 22 18:39:15 UTC 2023


On Wed, 22 Nov 2023 08:48:09 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> David Holmes has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
>> 
>>  - Merge with master and update Zero code accordingly
>>  - Merge branch 'master' into 8318776-supports_cx8
>>  - Remove unnecessary includes of vm_version.hpp.
>>    Fix copyright years.
>>  - Remove cx8 comment as no longer relevant (the spinlock is used regardless of cx8)
>>  - Remove suports_cx8() checks from gtest
>>  - Remove test for VMSupportsCX8
>>  - 8318776: Require supports_cx8 to always be true
>
> src/hotspot/cpu/x86/vm_version_x86.cpp line 819:
> 
>> 817:   }
>> 818: 
>> 819:   _supports_cx8 = supports_cmpxchg8();
> 
> I think we should leave the runtime check here (under `ifndef`, like in ARM?). This covers the remaining case of running on legacy x86 without CX8 implemented: the init guarantee would then fire and prevent any other surprises at runtime. Sure, it would be hard to come up with such a platform today, but it would be safer to refuse to run there right away on the off-chance someone actually has it :)

@shipilev - Do you have a particular legacy x86 in mind?

> src/hotspot/share/runtime/vm_version.cpp line 33:
> 
>> 31: void VM_Version_init() {
>> 32:   VM_Version::initialize();
>> 33:   guarantee(VM_Version::supports_cx8(), "Support for 64-bit atomic operations in required in this release");
> 
> Typo: "in required in". Also, no need to mention "this release" at all?
> Suggestion for message: "JVM requires platform support for 64-bit atomic operations"

Or the simpler change:
s/in required/is required/

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16625#discussion_r1402515036
PR Review Comment: https://git.openjdk.org/jdk/pull/16625#discussion_r1402528045


More information about the core-libs-dev mailing list