Issues / #1488

#1488 prefill.cpp: a GCC 10 build fails (std::atomic::wait is a GCC 11 library feature, with no guard and no documented minimum)

open · @zhouxihong1 · 0 comentários · No GitHub

Setup & installMulti-GPUAMD / HIPNVIDIA / CUDADocumentation

Descrição

## Problem

Building the engine with **GCC 10** fails in `src/prefill/prefill.cpp`:

```
[90/136] Building CXX object CMakeFiles/strata_prefill.dir/src/prefill/prefill.cpp.o
FAILED: CMakeFiles/strata_prefill.dir/src/prefill/prefill.cpp.o
/usr/bin/g++-10 ... -std=c++2a ... -c src/prefill/prefill.cpp
src/prefill/prefill.cpp:436:104: error: 'struct std::atomic<int>' has no member named 'wait'
  436 |  for (int i; (i = issued.load(std::memory_order_acquire)) <= j - kRing;) issued.wait(i);
src/prefill/prefill.cpp:488:16: error: 'struct std::atomic<int>' has no member named 'notify_all'
src/prefill/prefill.cpp:494:16: error: 'struct std::atomic<int>' has no member named 'notify_all'
```

`std::atomic<T>::wait` / `notify_one` / `notify_all` (C++20, P1135R6 — the synchronization library) is a **library** feature: **libstdc++ ships it from GCC 11**. GCC 10's `<atomic>` has no such member, so this is not something `-std=` can fix.

It came in with #1057 ("let Stager threads sleep instead of yield-spinning"), whose notes say: *"the ring wait uses C++20 `std::atomic::wait`, with `notify_all` in `issued_one()` and `finish()`"*. That work was measured on Ubuntu 24.04 (GCC 13), so GCC 10 was never tried. It is in 0.1.40.2 and 0.1.40.3.

**There is no compile-time guard, and the runtime switch cannot help:** `STRATA_STAGER_SLEEP` (`prefill.cpp:124`) is read with `std::getenv`, so a GCC 10 build cannot get past the three call sites — the option only picks the spin path *after* the code compiles.

## Why I think this is a portability gap, not just a doc nit

- `CMakeLists.txt:51-53` sets `CMAKE_CXX_STANDARD 20` + `CMAKE_CXX_STANDARD_REQUIRED ON`, and **GCC 10 satisfies that** — configure succeeds, so the failure reads like a code bug rather than a missing requirement.
- `docs/INSTALL.md:69` says the engine needs "a C++ compiler and git: build-essential" — no version.
- `setup.py:2458` prints `sudo apt install build-essential git` — no version, and nothing checks one.
- `CMakeLists.txt:1043-1045` still documents **GCC < 11** as a supported (if slower) configuration: *"A compiler without them (GCC < 11, Clang < 12) builds the files without the copies."*
- **Ubuntu 20.04 LTS (focal) offers at most GCC 10 in its own repositories; GCC 11 is not in focal at all** (packages.ubuntu.com lists `gcc-11` for jammy and later only). Focal is still in ESM until 2030, and Debian 11 (bullseye) also has GCC 10 as the newest in its base repos. Anyone on those needs a third-party PPA (`ppa:ubuntu-toolchain-r/test`) before the tree compiles at all.

So the effective minimum moved from GCC 10 to GCC 11 in 0.1.40.2 without a guard, a check, or a line of documentation.

## Verified

- **GCC 11 fixes it, changing nothing else.** Same tree, same CUDA Toolkit 12.2, same `--cuda 12`, `-DCMAKE_CUDA_ARCHITECTURES=89`: `g++-11` builds, `g++-10` does not. Purely the compiler version.
- Once GCC 11 was installed the whole engine built and the CUDA 12.2 path is fine (the `vmm.cpp` case from #1071 is solved here too, thanks).
- Machine: Ubuntu 20.04, 4x RTX 4090 (sm_89), CUDA Toolkit 12.2, `--cuda 12`, `--layer-split 12,24,36`.

## Suggested fixes (either is fine)

**1. State the minimum.** A check next to the existing `CMAKE_CUDA_ARCHITECTURES` ones, plus one line in `docs/INSTALL.md` and in the `build-essential` message. Then a GCC 10 user gets one clear message instead of three compile errors.

**2. Guard the three call sites** with the standard feature-test macro and keep the `yield` spin where it is unavailable — that is exactly the pre-#1057 behaviour, and `notify_all()` is redundant on the spin path:

```diff
--- a/src/prefill/prefill.cpp
+++ b/src/prefill/prefill.cpp
@@ -432,9 +432,11 @@
                 if (j >= kRing) {   // job j - kRing's DMA from this buffer is queued
+#if defined(__cpp_lib_atomic_wait)
                     if (stager_sleep())
                         for (int i; (i = issued.load(std::memory_order_acquire)) <= j - kRing;) issued.wait(i);
                     else
+#endif
                         while (issued.load(std::memory_order_acquire) <= j - kRing) std::this_thread::yield();
                 }
@@ -485,13 +487,17 @@
     void issued_one(int j, cudaStream_t copy) {
         cudaEventRecord(dma_done[j % kRing], copy);
         issued.store(j + 1, std::memory_order_release);
+#if defined(__cpp_lib_atomic_wait)
         issued.notify_all();
+#endif
     }
     /// No job is running after this (the end of a layer, or an early return in the middle of one).
     void finish() {
         head.store((uint64_t) gen << 32, std::memory_order_release);   // n = 0: nothing more to claim
         issued.store(1 << 30, std::memory_order_release);
+#if defined(__cpp_lib_atomic_wait)
         issued.notify_all();
+#endif
         while (active.load(std::memory_order_acquire) != 0) std::this_thread::yield();
     }
```

`__cpp_lib_atomic_wait` is the standard feature-test macro: GCC 11+ defines it and keeps today's behaviour unchanged; GCC 10 falls back to the spin. I applied this locally (6 insertions, one file, `git apply --check` clean against v0.1.40.3) and the tree then builds with GCC 10 — but it is only a workaround for the missing version statement, which is why I would prefer fix 1 with 2 as well.

Related: #1101 touches the same `Stager` waits.

No site

Links install, modelos, releases.