Use __atomic builtins as the freelist 128-bit CAS fallback - #13571
Open
phongn wants to merge 1 commit into
Open
Conversation
Platforms without an inline 128-bit CAS, such as riscv64, fail the build with "unsupported processor". Neither GCC nor LLVM emit an inline 128-bit CAS on riscv64, even with the Zacas extension, so a hand-written pointer-packing branch would be the only alternative and would depend on the kernel's virtual address width. Fall back to the __atomic builtins instead. They lower to libatomic calls, which may take internal locks; that is correct because every access to a shared head_p goes through INK_QUEUE_LD and ink_atomic_cas. Also revive the orphaned atomic list stress test as Catch2 tests and remove the dead INK_QUEUE_NT code. Fixes: apache#13555 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 19, 2026
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
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.
Summary
ATS does not build on riscv64. The build stops in
ink_queue.hwith the error "unsupported processor" (#13555). The freelist head needs one of two mechanisms: a 128-bit CAS, or a hand-written pointer layout with version bits. riscv64 has neither mechanism.This change adds a portable fallback. When the 16-byte
__syncbuiltins are not available, the freelist uses the__atomicbuiltins. The__atomicbuiltins lower to libatomic calls. The build then links against libatomic.Why not wait for Zacas hardware
The Zacas extension does not help here:
amocas.qinstruction. The initial Zacas support deferred it, and it never landed.amocas.qfor ABI-compatibility reasons. See the LLVM RISC-V usage notes.Thus, on riscv64 the 16-byte atomic operations always go through libatomic. This is true on current hardware and on future Zacas hardware.
Why not a hand-written pointer layout
A packed 64-bit layout is possible, but it is fragile on RISC-V:
LogObject.ccuses the version field as a reference count, so few version bits are not safe.The
__atomicfallback has no address-space assumptions and keeps the full 64-bit version field. A packed riscv64 fast path can come later as an optimization if measurements justify it.Correctness
On riscv64, libatomic implements the 16-byte operations with internal locks. This is correct for this code because every access to a shared
head_pgoes throughINK_QUEUE_LDandink_atomic_cas. All of these operations serialize on the same libatomic lock, so the load/CAS retry loops keep their current semantics. The code comments now record this constraint.The
__syncpath stays first in the probe order. x86-64 and aarch64 builds produce the same code as before; GCC does not inline the 16-byte__atomicCAS even with-mcx16(GCC PR80878), so a blanket switch would regress them.Note: libatomic exports only
__atomic_*symbols. The 16-byte__syncbuiltins emit undefined__sync_*_16references when the compiler cannot inline them, and nothing provides those symbols. That is why the fallback must use the__atomicbuiltins, and why the two paths are mutually exclusive.Changes
cmake/Check128BitCas.cmake: probe the__atomicbuiltins when the__syncprobe fails, first without and then with-latomic. New variables:TS_HAS_128BIT_CAS_LIBATOMIC,TS_NEEDS_LIBATOMIC_FOR_CAS. GCC >= 14 removed the automatic-latomicon RISC-V, so the explicit link is necessary.ink_queue.h:INK_QUEUE_LDbecomes a 16-byte__atomic_loadon the fallback tier;head_pkeeps the plain{pointer, int64 version}layout there.ink_atomic.h: anink_atomic_cas<__int128_t>specialization uses__atomic_compare_exchange_non the fallback tier.src/tscore/CMakeLists.txt: linkatomic(PUBLIC) when needed.src/tscore/test_atomic.ccstress test was orphaned (no build system referenced it). It is replaced with Catch2 tests inunit_tests/test_InkAtomicList.cc: a concurrent push/pop/popall conservation test with a double-reachability detector, anink_atomiclist_removetest, and a concurrentInkFreeListnew/free test.INK_QUEUE_NTconditional (ink_queue_nt.cno longer exists) and report the new feature flag intraffic_layout.Performance
The fallback affects only platforms that had no working build before. A microbenchmark of the freelist access pattern (x86 proxy numbers): the lock-based path is ~1.7x slower than the inline CAS when uncontended (~37 ns vs ~21 ns per op), and equal or slightly faster under contention, because the futex parks waiters instead of burning CAS retries.
ProxyAllocatorthread caches absorb most freelist traffic, so the global head is not on the per-request fast path.Testing
test_tscorepass on x86-64 (__synctier, unchanged codegen).traffic_layout info --featuresshould showTS_HAS_128BIT_CAS_LIBATOMIC: 1there.Fixes: #13555
🤖 Generated with Claude Code