[libclc] Use CLC atomic functions for legacy OpenCL atom/atomic builtins #168325
Conversation
Main changes:
* OpenCL legacy atom/atomic builtins now call CLC atomic functions (which
use Clang __scoped_atomic_*), replacing previous Clang __sync_* functions.
* Change memory order from seq_cst to relaxed; keep device scope (spec
permits broader than workgroup). LLVM IR for _Z8atom_decPU3AS1Vi:
Before:
%2 = atomicrmw volatile sub ptr subrspace(1) %0, i32 1 syncscope("agent") seq_cst
After:
%2 = atomicrmw volatile sub ptr subrspace(1) %0, i32 1 syncscope("agent") monotonic
* Also adds OpenCL 1.0 atom_* variants without volatile on the pointer.
They are added for backward compatibility.
There was a problem hiding this comment.
Pull Request Overview
This PR migrates OpenCL legacy atomic builtins (atom_* and atomic_*) from deprecated Clang __sync_* functions to CLC atomic functions that use Clang __scoped_atomic_* intrinsics. The memory ordering is changed from sequential consistency to relaxed ordering while maintaining device scope, which produces more efficient LLVM IR (monotonic instead of seq_cst).
Key changes:
- Replace
__sync_*function calls with__clc_atomic_*equivalents using__ATOMIC_RELAXEDmemory order - Add non-volatile pointer overloads for backward compatibility with OpenCL 1.0
- Remove obsolete helper functions and include files that are no longer needed
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| atomic_xor.cl | Replace __sync_fetch_and_xor with __clc_atomic_fetch_xor using relaxed ordering |
| atomic_xchg.cl | Replace __sync_swap_4 with __clc_atomic_exchange and unify float handling through macro |
| atomic_sub.cl | Replace __sync_fetch_and_sub with __clc_atomic_fetch_sub using relaxed ordering |
| atomic_or.cl | Replace __sync_fetch_and_or with __clc_atomic_fetch_or using relaxed ordering |
| atomic_min.cl | Replace __sync_fetch_and_min with __clc_atomic_fetch_min using relaxed ordering |
| atomic_max.cl | Replace __sync_fetch_and_max with __clc_atomic_fetch_max using relaxed ordering |
| atomic_cmpxchg.cl | Replace __sync_val_compare_and_swap with __clc_atomic_compare_exchange using relaxed ordering |
| atomic_and.cl | Replace __sync_fetch_and_and with __clc_atomic_fetch_and using relaxed ordering |
| atomic_add.cl | Replace __sync_fetch_and_add with __clc_atomic_fetch_add using relaxed ordering |
| atom_xor.cl | Replace __sync_fetch_and_xor_8 with __clc_atomic_fetch_xor and add non-volatile overloads |
| atom_xchg.cl | Replace __sync_swap_8 with __clc_atomic_exchange and add non-volatile overloads |
| atom_sub.cl | Replace __sync_fetch_and_sub_8 with __clc_atomic_fetch_sub and add non-volatile overloads |
| atom_or.cl | Replace __sync_fetch_and_or_8 with __clc_atomic_fetch_or and add non-volatile overloads |
| atom_min.cl | Replace custom __clc__sync_fetch_and_*min* helpers with __clc_atomic_fetch_min and add non-volatile overloads |
| atom_max.cl | Replace custom __clc__sync_fetch_and_*max* helpers with __clc_atomic_fetch_max and add non-volatile overloads |
| atom_inc.cl | Replace atomic_inc call with __clc_atomic_inc and add non-volatile overloads |
| atom_dec.cl | Replace atomic_dec call with __clc_atomic_dec and add non-volatile overloads |
| atom_cmpxchg.cl | Replace __sync_val_compare_and_swap_8 with __clc_atomic_compare_exchange and add non-volatile overloads |
| atom_and.cl | Replace __sync_fetch_and_and_8 with __clc_atomic_fetch_and and add non-volatile overloads |
| atom_add.cl | Replace __sync_fetch_and_add_8 with __clc_atomic_fetch_add and add non-volatile overloads |
| minmax_helpers.ll | Remove obsolete LLVM IR helpers for 64-bit min/max atomics |
| SOURCES | Remove reference to deleted minmax_helpers.ll |
| atom_decl_int64.inc | Remove redundant includes (clcfunc.h and clctypes.h) |
| atom_decl_int32.inc | Remove redundant includes (clcfunc.h and clctypes.h) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include <clc/opencl/atomic/atom_xor.h> | ||
| #include <clc/opencl/atomic/atomic_xor.h> | ||
|
|
||
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. |
There was a problem hiding this comment.
Corrected grammar: 'overloads is' should be 'overload is' (or 'overloads are' for plural).
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. | |
| // Non-volatile overloads are for backward compatibility with OpenCL 1.0. |
| #include <clc/opencl/atomic/atom_xchg.h> | ||
| #include <clc/opencl/atomic/atomic_xchg.h> | ||
|
|
||
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. |
There was a problem hiding this comment.
Corrected grammar: 'overloads is' should be 'overload is' (or 'overloads are' for plural).
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. | |
| // Non-volatile overloads are for backward compatibility with OpenCL 1.0. |
| #include <clc/opencl/atomic/atom_sub.h> | ||
| #include <clc/opencl/atomic/atomic_sub.h> | ||
|
|
||
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. |
There was a problem hiding this comment.
Corrected grammar: 'overloads is' should be 'overload is' (or 'overloads are' for plural).
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. | |
| // Non-volatile overloads are for backward compatibility with OpenCL 1.0. |
| #include <clc/opencl/atomic/atom_or.h> | ||
| #include <clc/opencl/atomic/atomic_or.h> | ||
|
|
||
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. |
There was a problem hiding this comment.
Corrected grammar: 'overloads is' should be 'overload is' (or 'overloads are' for plural).
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. | |
| // Non-volatile overloads are for backward compatibility with OpenCL 1.0. |
| #include <clc/opencl/atomic/atom_min.h> | ||
| #include <clc/opencl/atomic/atomic_min.h> | ||
|
|
||
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. |
There was a problem hiding this comment.
Corrected grammar: 'overloads is' should be 'overload is' (or 'overloads are' for plural).
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. | |
| // Non-volatile overloads are for backward compatibility with OpenCL 1.0. |
| #include <clc/opencl/atomic/atom_inc.h> | ||
| #include <clc/opencl/atomic/atomic_inc.h> | ||
|
|
||
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. |
There was a problem hiding this comment.
Corrected grammar: 'overloads is' should be 'overload is' (or 'overloads are' for plural).
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. | |
| // Non-volatile overloads are for backward compatibility with OpenCL 1.0. |
| #include <clc/opencl/atomic/atom_sub.h> | ||
| #include <clc/opencl/atomic/atomic_dec.h> | ||
|
|
||
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. |
There was a problem hiding this comment.
Corrected grammar: 'overloads is' should be 'overload is' (or 'overloads are' for plural).
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. | |
| // Non-volatile overloads are for backward compatibility with OpenCL 1.0. |
| #include <clc/opencl/atomic/atom_cmpxchg.h> | ||
| #include <clc/opencl/atomic/atomic_cmpxchg.h> | ||
|
|
||
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. |
There was a problem hiding this comment.
Corrected grammar: 'overloads is' should be 'overload is' (or 'overloads are' for plural).
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. | |
| // Non-volatile overloads are for backward compatibility with OpenCL 1.0. |
| #include <clc/opencl/atomic/atom_and.h> | ||
| #include <clc/opencl/atomic/atomic_and.h> | ||
|
|
||
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. |
There was a problem hiding this comment.
Corrected grammar: 'overloads is' should be 'overload is' (or 'overloads are' for plural).
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. | |
| // Non-volatile overloads are for backward compatibility with OpenCL 1.0. |
| #include <clc/opencl/atomic/atom_add.h> | ||
| #include <clc/opencl/atomic/atomic_add.h> | ||
|
|
||
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. |
There was a problem hiding this comment.
Corrected grammar: 'overloads is' should be 'overload is' (or 'overloads are' for plural).
| // Non-volatile overloads is for backward compatibility with OpenCL 1.0. | |
| // Non-volatile overloads are for backward compatibility with OpenCL 1.0. |
| #include <clc/opencl/atomic/atom_add.h> | ||
| #include <clc/opencl/atomic/atomic_add.h> | ||
|
|
||
| // Non-volatile overloads are for backward compatibility with OpenCL 1.0. |
There was a problem hiding this comment.
I don't remember the history here, but isn't this backwards? Even if the language is volatile qualified we probably shouldn't be emitting IR with volatile markers
There was a problem hiding this comment.
I don't remember the history here, but isn't this backwards?
OpenCL 1.0 atom_* builtins' pointer argument doesn't have volatile.
Even if the language is volatile qualified we probably shouldn't be emitting IR with volatile markers
Current implementation of CLC and OpenCL atomic function only have volatile in builtin function mangling. In LLVM IR, volatile is dropped, see the casting (ADDRSPACE __CLC_CASTTYPE *)Ptr that discards volatile at
So it should match with your comment.
3 participants
Main changes:
Before:
%2 = atomicrmw volatile sub ptr subrspace(1) %0, i32 1 syncscope("agent") seq_cst
After:
%2 = atomicrmw volatile sub ptr subrspace(1) %0, i32 1 syncscope("agent") monotonic