RFR: 8360654: AArch64: Remove redundant dmb from C1 compareAndSet
Ruben
duke at openjdk.org
Wed Feb 4 16:55:53 UTC 2026
On Wed, 4 Feb 2026 13:13:05 GMT, Andrew Haley <aph at openjdk.org> wrote:
>> This is duplicate of the PR #26000 which was originally created by @spchee.
>>
>> ===========
>>
>> AtomicLong.CompareAndSet has the following assembly dump snippet which gets emitted from the intermediary LIRGenerator::atomic_cmpxchg:
>>
>>
>> ;; cmpxchg {
>> 0x0000e708d144cf60: mov x8, x2
>> 0x0000e708d144cf64: casal x8, x3, [x0]
>> 0x0000e708d144cf68: cmp x8, x2
>> ;; 0x1F1F1F1F1F1F1F1F
>> 0x0000e708d144cf6c: mov x8, #0x1f1f1f1f1f1f1f1f
>> ;; } cmpxchg
>> 0x0000e708d144cf70: cset x8, ne // ne = any
>> 0x0000e708d144cf74: dmb ish
>>
>> According to the Oracle Java Specification, AtomicLong.CompareAndSet [1] has the same memory effects as specified by VarHandle.compareAndSet which has the following effects: [2]
>>
>>> Atomically sets the value of a variable to the
>>> newValue with the memory semantics of setVolatile if
>>> the variable's current value, referred to as the witness
>>> value, == the expectedValue, as accessed with the memory
>>> semantics of getVolatile.
>>
>> Hence the release on the store due to setVolatile only occurs if the compare is successful. Since casal already satisfies these requirements, the dmb does not need to occur to ensure memory ordering in case the compare fails and a release does not happen.
>>
>> Hence we remove the dmb from both casl and casw (same logic applies to the non-long variant)
>>
>> This is also reflected by C2 not having a dmb for the same respective method.
>>
>> [1] https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/util/concurrent/atomic/AtomicLong.html#compareAndSet(long,long)
>> [2] https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/lang/invoke/VarHandle.html#compareAndSet(java.lang.Object...)
>
> This is from atomicAccess.hpp:
>
>
> enum atomic_memory_order {
> // The modes that align with C++11 are intended to
> // follow the same semantics.
> memory_order_relaxed = 0,
> memory_order_acquire = 2,
> memory_order_release = 3,
> memory_order_acq_rel = 4,
> memory_order_seq_cst = 5,
> // Strong two-way memory barrier.
> memory_order_conservative = 8
> };
>
>
> So we could perhaps do:
>
> enum {
> CAS_ACQUIRE = 1 << memory_order_acquire,
> CAS_RELEASE = 1 << memory_order_release,
> ...
> CAS_CONSERVATIVE = 1 << memory_order_conservative,
> CAS_WEAK = 1 << 16
> };
>
>
> I'd encourage you to try to reduce the complexity of what we have, then add strong barrier (which is effectively memory_order_conservative) semantics.
Thanks for the guidance, @theRealAph.
I understand the technical debt needs to be addressed before this change can be merged.
I plan to work on refactoring based on the approach you suggested, but I might not be able to schedule this before April.
> A patch to do this will be far more extensive than this PR, but we can't go any further without paying back technical debt.
Sure, I can open a separate PR for the refactoring and then rebase this PR on top of it.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/29287#issuecomment-3848574588
More information about the hotspot-dev
mailing list