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