Skip to content

Make MP:CAS on an SVREF place actually atomic - #1838

Open
dg1sbg wants to merge 1 commit into
clasp-developers:mainfrom
dg1sbg:fix/cas-svref-atomic
Open

Make MP:CAS on an SVREF place actually atomic#1838
dg1sbg wants to merge 1 commit into
clasp-developers:mainfrom
dg1sbg:fix/cas-svref-atomic

Conversation

@dg1sbg

@dg1sbg dg1sbg commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

MP:CAS on an SVREF place compiles, is correct single-threaded, and does not arbitrate
between threads. Four threads running 50000 compare-and-swap increments each land
171000-182000 of 200000 — 9-15% of the updates silently lost, on every run.

This is worse than a refusal, because every single-threaded test passes. SVREF has been
the documented place that works for portable compare-and-swap, so code built on that
reading is silently unsound.

Why

The SVREF atomic expander (src/lisp/kernel/lsp/atomics.lisp:349-367) emits CORE:ACAS,
which has two implementations:

  • Nativebir-to-bmir.lisp:426 transforms the call to the CORE:ACAS primop and
    primop.lisp:679 lowers it to an LLVM cmpxchg. Genuinely atomic.
  • C++ fallback core__acas (src/core/array.cc) — a plain aref, an EQ compare and
    a plain aset, with the order discarded. Its own comment said so: "BUT: They aren't
    actually atomic."

The fallback is not a corner case. The bytecode VM has no atomic opcode at all, so besides
every --build-mode=bytecode build, EVAL, the REPL and LOAD of a source file reach it
in every build. CORE:ATOMIC-AREF and its SETF had the identical defect.

Measured in one process before the change:

arm result
interpreted LOST 8366 / 80000
bytecode LOST 12458 / 80000
native (asserted SIMPLE-CORE-FUN) exact

The fix

A simple vector stores elements in a plain GCArray_moveable<T_sp>, so unlike Rack
which stores GCArray_atomic<T_sp>, and is why every CLOS-side casser already arbitrated —
there is no std::atomic to call. The three entry points now operate on the element's
tagged word with the atomic builtins.

This needs no GC cooperation: Clasp has no write barrier of any kind, no GC variant
relocates objects (Boehm is mark-sweep with interior pointers; the MMTk variant runs Immix
with copying compiled out), the only relocating operation is snapshot save and it runs with
the world stopped, and gc_yield is called only on function entry so no safepoint can fall
inside the swap. It is also precisely what the native path already emits — verified by
disassembly: one inline casal, and llvm-nm -u shows no __atomic_* libcall.

All three resolve the address through asAbstractSimpleVectorRange, so displaced and
fill-pointer general arrays keep working, and the row-major index is computed once instead
of twice.

Behaviour change worth reviewing

CORE:ACAS on storage that is not a simple vector of T now signals a TYPE-ERROR of
expected type (ARRAY T), where it previously performed a non-atomic read-compare-write
that could not have succeeded anyway — a freshly boxed element is never EQ to the
expected value.

The refusal is deliberately asymmetric and mirrors the native path's own asymmetry:
bir-to-bmir.lisp transforms CORE:ACAS only for SIMPLE-VECTOR, so refusing other
storage makes the two implementations agree on their domain; but it transforms
CORE:ATOMIC-AREF for single-float, double-float, base-char and character arrays
too, so making the reader and writer refuse those would have made the same source succeed
natively and error in bytecode. They keep their existing behaviour instead.

A static_assert requires the tagged word to swap lock-free, so a target without an
8-byte lock-free CAS fails the build rather than quietly emitting a libatomic call that
takes a lock inside the swap window.

Tests

Seven tests in the existing mp suite. The "out-of-line" variants reach CORE:ACAS
through FDEFINITION so the runtime implementation is exercised even where the compiler
would inline a cmpxchg instead. They cover contention, the single-threaded and
single-swap controls, displacement, and the refusal.

They are a real gate: against the pre-fix binary 3 of the 7 fail; after the fix all 7 pass.
The 4 that pass pre-fix are the controls, which must keep passing — a "fix" that made every
swap succeed, or every swap refuse, would satisfy the arithmetic and fail those.

Verification

macOS arm64, boehmprecise, plain Clasp (no extensions), built from this branch's parent
plus this commit:

  • Reproducer: 5/5 runs exit 0 at exactly 200000/200000.
  • All four arms exact at 4x50000: interpreted, bytecode, native, and the direct
    out-of-line call.
  • Regression suite: 2004 successes = 1997 baseline + these 7 tests; failures are exactly
    the 5 known ones by name (SBCL-CROSS-COMPILE-4, INCLUDE-LEVEL-2B, INCLUDE-LEVEL-3,
    WEAK-KEY-OR-VALUE-WEAKNESS, TYPES-CLASSES-10).
  • ANSI: 21936 tests, 27 failures, 0 unexpected.
  • clang-format clean.

Reported as dg1sbg#1, which carries the standalone reproducer.

MP:CAS on (SVREF v i) compiled, and was correct single-threaded, and did not
arbitrate between threads. Four threads running 50000 compare-and-swap
increments each landed 171000-182000 of 200000, losing 9-15% of the updates,
on every run.

The atomic expander for SVREF in src/lisp/kernel/lsp/atomics.lisp emits
CORE:ACAS, and CORE:ACAS has two implementations. The native one is a genuine
compare-and-swap: bir-to-bmir.lisp transforms the call to the CORE:ACAS
primop and primop.lisp lowers that to an LLVM cmpxchg on the element address.
The other is the C++ fallback core__acas in src/core/array.cc, taken whenever
the Cleavir transform does not run. That is not a corner: the bytecode VM has
no atomic opcode at all, so besides every --build-mode=bytecode build, EVAL,
the REPL and LOAD of a source file reach the fallback in *every* build. That
fallback was a plain aref, an EQ compare and a plain aset, as its own comment
admitted. Measured on this tree before the change: the native arm lands
exactly, the bytecode and interpreted arms both lose updates.

A simple vector stores its elements in a plain GCArray_moveable<T_sp>, so
unlike Rack -- which stores GCArray_atomic<T_sp>, and is why every CLOS-side
casser already arbitrated -- there is no std::atomic to call. Operate on the
element's tagged word directly with the atomic builtins instead. That needs
no GC cooperation: Clasp has no write barrier of any kind, no GC variant
relocates objects (Boehm is mark-sweep with interior pointers, the MMTk
variant runs Immix with copying compiled out), the only relocating operation
is snapshot save and it runs with the world stopped, and gc_yield is called
only on function entry so no safepoint can fall inside the swap. It is also
precisely what the native path already emits.

CORE:ATOMIC-AREF and (SETF CORE:ATOMIC-AREF) told the same lie and are fixed
the same way. All three now resolve the element address through
asAbstractSimpleVectorRange, so displaced and fill-pointer general arrays
keep working, and as a side effect the row-major index is computed once
instead of twice. Specialized arrays have no tagged word to swap: the reader
and writer fall back to the ordinary accessors exactly as before, while ACAS
signals a TYPE-ERROR instead of silently performing a non-atomic
read-compare-write that could not have succeeded anyway, since a freshly
boxed element is never EQ to the expected value.

As with core__car_atomic in cons.cc these ignore the requested memory order
and use the strongest one. That is always valid, and this is by construction
the out-of-line path.

A static_assert requires the tagged word to swap lock-free. Were that ever
untrue on some target the builtin would quietly emit a libatomic call, which
takes a lock inside the swap window; failing the build is the better outcome.

The refusal is deliberately asymmetric, and mirrors the native path's own
asymmetry: bir-to-bmir.lisp transforms CORE:ACAS only for SIMPLE-VECTOR, so
ACAS refusing other storage makes the two implementations agree on their
domain, whereas it transforms CORE:ATOMIC-AREF for single-float, double-float,
base-char and character arrays too, so making the reader and writer refuse
those would have made the same source succeed natively and error in bytecode.
They keep their existing behaviour instead.

The new tests in the mp suite reach CORE:ACAS through FDEFINITION so that the
runtime implementation is exercised even in a build whose compiler would
inline a cmpxchg for the MP:CAS form instead. They cover contention, the
single-threaded and single-swap controls, displacement, and the refusal of
specialized arrays.

Fixes #1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant