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

[Aleksey Shipilev]

On Wed, 22 Nov 2023 02:09:38 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> As discussed in JBS all platforms (some tweaks to Zero are in progress) actually do support `cx8` i.e. 64-bit compare-and-exchange, so we can strip out the locked-based alternatives to using it and just add a guarantee that it is true at runtime. And all platforms except some ARM variants set `SUPPORTS_NATIVE_CX8`, so we can greatly simplify things. Summary of changes:
>> - `_supports_cx8` field is only needed when `SUPPORTS_NATIVE_CX8` is not defined
>> - Assertions for `supports_cx8()` are removed
>> - Compiler predicates requiring `supports_cx8()` are removed
>> - Access backend is greatly simplified without the need for lock-based alternative
>> - `java.util.concurrent.AtomicLongFieldUpdater` is simplified without the need for a lock-based alternative
>> 
>> I did consider moving all the ARM `kuser_helper` related code to be only defined when `SUPPORTS_NATIVE_CX8` is not defined, but there was a theoretical risk this could change the behaviour if ARMv7 binaries were run on other ARM CPU's. I added a note to that effect in vm_version_linux_arm32.cpp so the ARM port maintainers could clean this up further if desired.
>> 
>> Testing:
>> - All Oracle tiers 1-5 builds (which includes an ARMv7 build)
>> - GHA builds/tests
>> - Oracle tiers 1-3 sanity testing
>> 
>> Zero changes coming in via [JDK-8319777](https://bugs.openjdk.org/browse/JDK-8319777) will be merged when they arrive.
>> 
>> Thanks.
>
> 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

Thanks! Zero tests are running. The PR looks great, except extra safety suggestion in x86 part:

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 :)

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

PR Review: https://git.openjdk.org/jdk/pull/16625#pullrequestreview-1743847107
PR Review Comment: https://git.openjdk.org/jdk/pull/16625#discussion_r1401696816