RFR: 8367013: Add Atomic<T> to package/replace idiom of volatile var plus AtomicAccess:: operations [v6]

[Johan Sjölen]

On Tue, 11 Nov 2025 06:26:01 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Please review this change that adds the type Atomic<T>, to use as the type
>> of a variable that is accessed (including writes) concurrently by multiple
>> threads. This is intended to replace (most) uses of the current HotSpot idiom
>> of declaring a variable volatile and accessing that variable using functions
>> from the AtomicAccess class.
>> https://github.com/openjdk/jdk/blame/528f93f8cb9f1fb9c19f31ab80c8a546f47beed2/doc/hotspot-style.md#L138-L147
>> 
>> This change replaces https://github.com/openjdk/jdk/pull/27462. Differences are
>> 
>> * Substantially restructured `Atomic<T>`, to be IDE friendly. It's
>> operationally the same, with the same API, hence uses and gtests didn't need
>> to change in that respect. Thanks to @stefank for raising this issue, and for
>> some suggestions toward improvements.
>> 
>> * Changed how fetch_then_set for atomic translated types is handled, to avoid
>> having the function there at all if it isn't usable, rather than just removing
>> it via SFINAE, leaving an empty overload set.
>> 
>> * Added more gtests.
>> 
>> Testing: mach5 tier1-6, GHA sanity tests
>
> Kim Barrett 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 20 additional commits since the last revision:
> 
>  - Merge branch 'master' into atomic-template-tag-select
>  - remove AtomicNextAccess and uses
>  - use type_traits wrapper in new code
>  - Merge branch 'master' into atomic-template-tag-select
>  - more naming updates
>  - rename relaxed_store => store_relaxed
>  - default construct translated atomic without SFINAE
>  - Merge branch 'master' into atomic-template-tag-select
>  - Merge branch 'master' into atomic-template-tag-select
>  - add reference to gcc bug we're working around
>  - ... and 10 more: https://git.openjdk.org/jdk/compare/99f95a49...da58d0d2

I found a couple of bugs where we forgot to pass along the memory order. Other than that, this seems good! Great job!

src/hotspot/share/runtime/atomic.hpp line 276:

> 274:   T compare_exchange(T compare_value, T new_value,
> 275:                      atomic_memory_order order = memory_order_conservative) {
> 276:     return AtomicAccess::cmpxchg(value_ptr(), compare_value, new_value);

Bug: This isn't providing the `order` to `cmpxchg`

src/hotspot/share/runtime/atomic.hpp line 291:

> 289:   T exchange(T new_value,
> 290:              atomic_memory_order order = memory_order_conservative) {
> 291:     return AtomicAccess::xchg(this->value_ptr(), new_value);

Bug: This isn't providing the order to cmpxchg

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

Marked as reviewed by jsjolen (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27539#pullrequestreview-3447626292
PR Review Comment: https://git.openjdk.org/jdk/pull/27539#discussion_r2513896902
PR Review Comment: https://git.openjdk.org/jdk/pull/27539#discussion_r2513898530