Conversation
This was referenced Sep 25, 2026
maleadt
added this pull request to stack #29
September 25, 2026 13:30
maleadt
marked this pull request as ready for review
September 25, 2026 15:11
The load, store, cmpxchg and atomicrmw templates for Ptr interpolated the ordering but never the scope, so calls with `singlethread` (and `none` on types that don't fit the intrinsics) silently used the system scope. The existing tests only compared values, so check the IR too.
The generic Ptr fallbacks bitcast to a same-sized unsigned integer and dispatch again. When `T` already is that integer, e.g. for an invalid ordering like `load(p, release)` or an unsupported scope, they called themselves until the stack overflowed. Throw a ConcurrencyViolationError for invalid orderings, like Base does, and an ArgumentError otherwise.
`cas!(p, cmp, new, order)` used `order` as the failure ordering too, but a failure ordering can't release: acq_rel and release gave a StackOverflowError on Ptr and a MethodError on LLVMPtr. Weaken it like C++ does instead: release becomes monotonic and acq_rel acquire.
On Ptr, modify! only worked for operations with an atomicrmw instruction for the value type: `max` on floats, Bool operands or arbitrary functions were a MethodError. LLVMPtr already falls back to a cmpxchg loop, so do the same for Ptr. Being computed in Julia, the result also has Julia's semantics, e.g. NaN propagation for `max`.
LLVM doesn't allow atomicrmw to be unordered, but the Ptr methods were generated for every ordering and the LLVMPtr path passed it through, so `add!(p, x, unordered)` failed to parse the IR. Throw the same ConcurrencyViolationError as for other invalid orderings.
The inline assembly was selected by the host architecture, so on an x86_64 host it also ends up in GPU kernels that call `fence()`, where it is invalid. Put it behind `Internal.cpu_seq_cst_fence()`, which is defined on every host, so that GPU back-ends can overlay it with a plain `fence seq_cst`.
LLVM 20 lowers a seq_cst fence on x86_64 to the same locked `or` as the inline assembly (llvm/llvm-project#106555), for every CPU target, so the workaround is only needed on Julia 1.12 and older.
maleadt
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack: #28 → this PR → #27 → #30.
Bug fixes for 0.3.3, without API changes. Most of these bugs exist because the
PtrandLLVMPtrimplementations have drifted apart, and because the tests only checked results, never the generated IR:Ptr, the scope was silently dropped:load(p, monotonic, singlethread)emitted a system-scope load.Ptr, unsupported scopes or invalid orderings recursed until they hit aStackOverflowError. They now throw a proper error.cas!(p, cmp, new, acq_rel)also usedacq_relas the failure ordering, which LLVM rejects. The failure ordering is now derived as in C++.unorderedread-modify-write operations generated IR that failed to parse.Ptrhad no compare-and-swap fallback, so operations likemaxon floats were aMethodError.lock orqworkaround forfence(seq_cst)was selected based on the host, so it ended up in GPU kernels, where ptxas rejects it.Each fix is a separate commit with a regression test.
Downstream: the x86 fence workaround now lives in
UnsafeAtomics.Internal.cpu_seq_cst_fence()and is only used before LLVM 20. CUDA.jl and AMDGPU.jl should override it with a plainfence seq_cst.