RFR: 8290324: Move atomic operations outside of os_xxx.hpp [v2]

[Kim Barrett]

On Sat, 16 Jul 2022 01:28:54 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> The os_xxx.hpp files inject extra methods/fields that are specific to atomic operations into the `os` class. However, the injected methods/fields are used only by a specific os/cpu combination. Therefore, they should not be inside the `os` class, which should contain only APIs that are used across platforms.
>> 
>> - For ports where the `atomic_copy64()` function is used in a single file, I moved it as an inline function in that file
>> - Otherwise it's moved to `atomic_<os>_<cpu>.hpp`
>> - The linux/arm port is a little more involved, but the new code should be a little cleaner than the old code.
>
> Ioi Lam 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 two additional commits since the last revision:
> 
>  - Merge branch 'master' into 8290324-move-atomic-ops-outside-os-hpp
>  - 8290324: Move atomic operations outside of os_xxx.hpp

src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 587:

> 585: 
> 586:   inline void atomic_copy64(const volatile void *src, volatile void *dst) {
> 587:     *(jlong *) dst = *(const jlong *) src;

s/jlong/uint64_t/ (or int64_t).  And casting away volatile seems kind of sketchy.  This could be

Atomic::store((uint64_t*)dst, Atomic::load((const uint64_t*)src));

which seems clearer to me.

src/hotspot/os_cpu/bsd_zero/atomic_bsd_zero.hpp line 312:

> 310:   STATIC_ASSERT(8 == sizeof(T));
> 311:   volatile int64_t dest;
> 312:   atomic_copy64(reinterpret_cast<const volatile int64_t*>(src), reinterpret_cast<volatile int64_t*>(&dest));

[pre-existing] Why do we have these casts when `atomic_copy64` takes `volatile void*`?  (There are more like this that I didn't bother to comment directly.)

src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp line 445:

> 443:       const jlong *end = from + count;
> 444:       while (from < end)
> 445:         atomic_copy64(from++, to++);

If `atomic_copy64` was a shared API with platform-specific implementations we could eliminate all these copies of `_Copy_conjoint_jlongs_atomic`.

src/hotspot/os_cpu/linux_arm/atomic_linux_arm.hpp line 65:

> 63: };
> 64: 
> 65: extern arm_atomic_funcs _arm_atomic;

Why is arm_atomic_funcs not an AllStatic class?  (And it has a non-conventional name.)

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

PR: https://git.openjdk.org/jdk/pull/9501