RFR: 8290324: Move atomic operations outside of os_xxx.hpp [v2]
Ioi Lam
iklam at openjdk.org
Mon Jul 18 21:37:20 UTC 2022
On Mon, 18 Jul 2022 20:54:22 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>> 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.
This is also from the original version. I think we can fix this in a separate PR.
> 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.)
To be honest I don't know :-)
-------------
PR: https://git.openjdk.org/jdk/pull/9501
More information about the hotspot-runtime-dev
mailing list