RFR: 8372528: Unify atomic exchange and compare exchange [v3]

[Stefan Karlsson]

On Wed, 26 Nov 2025 10:20:14 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> AtomicAccess::xchg is only required to support `4` bytes and `sizeof(intptr_t)` size.
>> This restriction added a lot of extra logic to the Atomic implementation because
>> we have a set of features we must support (including compare exchange) for `1`, `4` and `8` byte atomics on all platforms. We have some checks for unsupported `8` byte compare exchange (`VM_Version::supports_cx8()`), but the Atomic class does not try to handle these for generating its supported functions. On such a platform we would more than likely get a linking error.
>> 
>> I propose we change requirement for exchange to `1`, `4` and `8` bytes to achieve parity with compare exchange. Initially by implementing exchange via the `AtomicAccess::XchgUsingCmpxch`. And have follow up RFEs for each applicable platform where we specialize `AtomicAccess::PlatformXchg<1>`.
>> 
>> This enhancement both simplifies the Atomic implementation and provides exchange capabilities for types like `bool` and enums represented by a byte.
>> 
>> _It is a little unclear how we deal with `VM_Version::supports_cx8()`. Its existence makes it impossible to use `compare_exchange` on `int64_t` in general code. Currently the `Atomic` implementation assumes that `exchange` can always be used on `8` byte integers (at least going by the gtest). Even though `AtomicAccess` only specifies `4` bytes and the platform size. This PR changes this to `1`, `4` and `8` bytes. But not sure if the previous behaviour / implicit requirements is an oversight a similar property to `VM_Version::supports_cx8()` should apply here for `exchange`._
>> 
>> * Testing
>>   * Extended gtest / (no other users of Atomic byte with exchange exists.
>>   * GHA
>>   * Running Tier 1-5 on Oracle supported platforms
>
> Axel Boldt-Christmas has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Remove VM_Version::supports_cx8() conditions
>  - Add AtomicAccessXchgTest for 1 byte

Looks good. I think I found one nit that you might want to clean out.

test/hotspot/gtest/runtime/test_atomic.cpp line 293:

> 291: 
> 292: TEST_VM(AtomicEnumTest, scoped_enum_64_bit) {
> 293:   // Check if 64-bit atomics are available on the machine.

This comment belonged to the supports_cx8 check:

  // Check if 64-bit atomics are available on the machine.
  if (!VM_Version::supports_cx8()) return;

and when that check has been removed the comment should probably also be removed.
Suggestion:


There's a similar left over below in the patch.

test/hotspot/gtest/runtime/test_atomicAccess.cpp line 144:

> 142: 
> 143: TEST_VM(AtomicAccessCmpxchgTest, int64) {
> 144:   // Check if 64-bit atomics are available on the machine.

Suggestion:

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

Marked as reviewed by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28498#pullrequestreview-3518855472
PR Review Comment: https://git.openjdk.org/jdk/pull/28498#discussion_r2571557166
PR Review Comment: https://git.openjdk.org/jdk/pull/28498#discussion_r2571557855