Use std::atomic, and delete the hand-written atomics

[d-torrance]

src/mathicgb/Atomic.hpp implemented atomic load and store by hand — compiler
barriers via __asm__ __volatile__ ("" ::: "memory"), CPU fences via
__sync_synchronize(), and a CAS loop on __sync_bool_compare_and_swap.
stdinc.h defined MATHICGB_USE_CUSTOM_ATOMIC_X86_X64 in both the _MSC_VER
and __GNUC__ branches, so on x86 and x64 — every ordinary Linux build — that,
and not std::atomic, is what MonomialMap and FixedSizeMonomialMap used.

Its own comment gave the premise to test:

it is surprising that any compiler ships with a std::atomic that is worse than
this - but that is very much the case.

Written in 2013 against GCC 4.x. It is no longer true, and the measurement is
not close.

What the compiler actually emits

Both implementations, same translation unit, GCC 13.3 at -O2, x86-64:

order	custom	std::atomic
load relaxed / acquire / seq_cst	movq	movq
store relaxed / release	movq	movq
store seq_cst	movq; lock cmpxchgq; jne	xchgq

Identical for every operation on the hot path, and on the one operation where
they differ the hand-written version is the worse of the two: a
compare-and-swap with a retry branch where std::atomic emits a single locked
exchange. So this is not a wash — it removes a slower implementation.

That seq_cst store is used once, at MonomialMap.hpp:188, on the map-growth
path. Traversal and insertion are relaxed, consume and release, which compile
the same either way — which is why timing cannot see the change: interleaved
runs of hyclic8-101-trimmed and cyclic7 at one and four threads, nine reps
each, land between −0.5% and +4.4% against 3–7% run-to-run deviation. Where the
emitted code is the same there is nothing to time.

The commits

1. 1503063 — delete the hand-written implementation. A pure deletion, 329
   lines, no additions.
2. ac7225e — remove MATHICGB_USE_FAKE_ATOMIC. It was added in c1105ec
   (2012-11-06) alongside the custom atomics, to measure their overhead. With
   those gone the whole difference between it and std::atomic is one
   instruction. It is also a quiet trap: nothing defines the macro, and a build
   that does passes 246/246 while running the suite multithreaded under TBB with
   no synchronization at all.
3. 84989af — remove Atomic.hpp itself and use std::atomic directly.
4. 3487b2c — update doc/description.txt, which described the deleted
   file and posed a project this branch answers.

On removing the wrapper, and why the comments changed

After the first two commits Atomic<T> only forwarded to a std::atomic
member, so the third commit removes it. It differed from std::atomic in two
ways, and one of them is load-bearing: Atomic() value-initialized, where
std::atomic's default constructor is trivial in C++17 and leaves the value
indeterminate. That is exactly why FixedSizeMonomialMap nulls its buckets by
hand.

Measured rather than assumed, by deleting the nulling from both constructors:

	with Atomic<T>	with plain std::atomic<T>
unit tests	246/246 pass	SIGSEGV

valgrind on the second: Conditional jump or move depends on uninitialised value(s) in F4MatrixBuilder2::Builder::findOrCreateColumn, under TBB —
garbage bucket pointers walked as a hash chain. So both constructors now say why
the nulling is required. The existing comment is kept as it was, minus one claim
that is no longer true: new std::atomic<void*>[64]() into deliberately dirtied
memory comes back fully null on GCC 13.3. Switching to that form would mean
changing make_unique_array, which PolyHashTable.cpp and mathicgb.cpp also
use, so the explicit nulling stays.

The alignment assertion went with the wrapper. It was load-bearing when the
member was a raw T and atomicity depended on alignment; a std::atomic aligns
itself, and alignof(std::atomic<void*>) is 8.

Verification

cmake Release and Debug both pass 246/246, autotools make check and make distcheck pass, and valgrind reports zero errors on an F4 run. Single-threaded
output is byte-identical to the old code on cyclic5, cyclic7,
hyclic8-101-trimmed and hilbertkunz1.

Multithreaded output cannot be compared that way, which is worth recording: the
same binary produces a different .gb on five consecutive cyclic7 runs, and
identical output at -threadCount 1. A first pass at this comparison looked
like a discrepancy between the two builds and was not one.

This is also the first branch to run under the 16-cell matrix from PR #80, which
is more than routine here: Atomic.hpp compiled differently on x86 than
anywhere else, so the macOS and arm64 cells are the interesting ones.

🤖 Generated with Claude Code