Skip to content

perf(cuda): split reg_flow into per-(family, ndim) translation units - #147

Open
balbasty wants to merge 9 commits into
mainfrom
perf/split-reg-flow-tus
Open

perf(cuda): split reg_flow into per-(family, ndim) translation units#147
balbasty wants to merge 9 commits into
mainfrom
perf/split-reg-flow-tus

Conversation

@balbasty

@balbastybalbasty commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Brought up to current main (247714e, through #153, #154 and #155) by merging rather than rebasing, so the push stays a fast-forward. src/lib-cuda/ had not been touched on main since this branch last merged, so the merge was clean, and the only code change since the last review is the one the review asked for.

What the review asked for

"ok but should use pragma once and the namespace macros"

#pragma once — already satisfied: both new headers (reg_flow_slice.h, reg_flow_slice.inl) open with it on line 1 and carry no #ifndef guard.

The namespace macros — was not satisfied, in exactly one file. reg_flow_slice.h spelled its namespaces by hand:

namespaceff {
namespacecuda {
namespaceflow_slice {

It is now FF_NAMESPACE_BEGIN(FF_NS) / (FF_DEVICE) / (flow_slice), pulling in core/cuda_switch.h for FF_DEVICE. The .inl and the twelve slice TUs already used the macros, so this was the odd file out rather than a pattern.

All four checkers pass --check: normalise-header-guards.py, normalise-include-delimiters.py, rename-macros.py (#145, #146, #91) and check-cuda-launches.py — and the lint (source conventions) job #156 has since added is green on this branch. I also confirmed the guard checker genuinely covers these two files rather than passing vacuously: injecting a whole-file guard into each makes it fail and name the file. check-cuda-launches.py matters here specifically because #154 landed the one-launch-site rule while this branch was open — the slice TUs launch nothing directly, they call the FF_CUHOST launchers in impl/cuda/reg_flow.h, and the lint still reports one <<< and 39 FF_CUDA_LAUNCH call sites.

Re-measured against #155 — and this is the part worth reading

#155 is why every number below was re-taken. It fixed nvcc replacing the body of every impl/kernels/utils.h helper in the host object with ::exit(1), with -O1 then deleting everything after the call. The first measurement of this split was taken against a library in that state.

The honest question is whether the split's case survives the fix. It does — and the finding is that the fix did not move the compile at all:

reg_flow, one TU, index32peak RSSnvcc elapsed
pre-#155 (run 32365351378, 85fdac7)12.98 GiB1149.89 s
post-#155 (run 32397332659, 0d40731)12.98 GiB1069.30 s

The peak is identical. That is not luck: the code #155 restored is host code, and the peak belongs to a device-side process (cicc/ptxas), so deleting host statements never touched the number that pins this build. A correctness catastrophe and a compile-cost non-event.

Every figure below is nonetheless from post-#155 runs on both sides — BEFORE 0d40731 (run 32397332659), AFTER 2c4842b (run 32418602148) — same image, same nvcc 12.0.140, same -O1, same bound/spline policy, -j2.

index32 — the default, and what ships

beforeafter
TUs for reg_flow113
heaviest reg_flow TU12.98 GiB1.94 GiB−85.0%
reg_flow elapsed, summed1069.30 s803.32 s−24.9%
make cuda -j2 wall2145 s1861 s−13.2%
max peak over any module12.98 (reg_flow)8.35 (reg_field)

Per slice, from the job's own FFMEM table:

reg_flow_matvec_3d 1.94 GiB 177.13 s reg_flow_matvec_1d 0.61 48.84
reg_flow_matvec_2d 1.22 GiB 98.53 s reg_flow_relax_2d 0.60 48.01
reg_flow_diag_3d 1.03 GiB 64.72 s reg_flow_kernel_2d 0.59 49.71
reg_flow_relax_3d 0.96 GiB 89.84 s reg_flow_kernel_1d 0.48 38.77
reg_flow_kernel_3d 0.89 GiB 83.19 s reg_flow_diag_1d 0.44 35.04
reg_flow_diag_2d 0.67 GiB 46.24 s reg_flow_relax_1d 0.26 21.66
reg_flow (front) 0.20 1.64

index64

beforeafter
heaviest reg_flow TU5.69 GiB1.05 GiB−81.5%
reg_flow elapsed, summed482.78 s455.23 s−5.7%
make cuda -j2 wall966 s918 s−5.0%
max peak over any module5.69 (reg_flow)3.76 (reg_field)

The elapsed saving is far smaller on this leg, which is the expected shape: collapsing the offset axis has already removed half the instantiations, so there is less left for the split to divide. Memory still falls by four fifths.

Two corrections to the earlier numbers

I am flagging these rather than quietly restating them.

  1. The time savings are smaller than first reported — −24.9% and −13.2%, not −34.6% and −25.5%. Same change, different runs.
  2. Wall-clock on this job is noisy and peak RSS is not.reg_field, which this PR does not touch, moved 8.23 → 8.35 GiB and 848.87 → 861.05 s between the same two runs. So read −13.2% as "clearly faster", not as a figure good to three digits. Peak RSS, by contrast, is solid: all thirteen per-slice peaks reproduce the earlier pre-fix(kernels): utils.h helpers must be host+device, not device-only (#150) #155 run of the same split to within 0.01 GiB, and reproduce off-runner — measured locally on a different machine at the same flags, the front TU came out 205532 kB against CI's 205332 (0.1%) and reg_flow_diag_1d 456904 against 456160 (0.2%).

What is here

filerole
reg_flow.cppall 15 exported entry points, every check, every message — and zero instantiations
reg_flow_slice.hthe seam: 30 hidden-visibility declarations, one per (entry point, ndim)
reg_flow_slice.inlthe five wrapper templates + the dtype × offset × bound dispatch, moved verbatim; bodies selected by FF_FLOW_SLICE_*
reg_flow_{matvec,diag,kernel,relax}_{1,2,3}d.cpptwelve four-line slice TUs

The cut is (operation family) × (ndim) because that is how the dispatch already factors: each entry point picks one internal wrapper template and switch (ndim)es it, and no two arms share an instantiation.

Why a forwarding seam, and not extern template or -D

Compiling one source N times under different -D — what BOUNDFLAGS does — gives N objects each defining ff::cuda::flow_matvec, and the link fails. So the entry point lives in one TU and the slices export something else.

extern template is not usable here: it must name every leaf, and with FF_STATIC_BOUNDS=0 six of the eight FF_BOUND_* selectors collapse onto bound::type::Dynamic, so the enumeration would name the same specialization six times — ill-formed under [temp.explicit]/5 — and which leaves collapse is a build-flag decision. A switch has no such problem: duplicate implicit instantiations across its arms are the same instantiation.

ABI — unchanged, and checkable per object

The slice functions are declared __attribute__((visibility("hidden"))), so they never reach .dynsym:

$ readelf -sW reg_flow_diag_1d.o | grep flow_slice
3 GLOBAL HIDDEN ff::cuda::flow_slice::{diag,adddiag,subdiag}_1d
$ readelf -sW reg_flow_relax_1d.o | grep flow_slice
1 GLOBAL HIDDEN ff::cuda::flow_slice::relax_1d

GLOBAL HIDDEN is the whole ABI argument: external linkage so the call in reg_flow.cpp resolves at link time, hidden visibility so the dynamic symbol table of libfastfields-cuda.so is what it was. The exported ff::cuda::flow_* entry points, their signatures, their argument checks and the order those fire in are untouched — the bodies changed file, not shape. (The demangling above is incidentally also what confirms FF_DEVICE still expands to cuda after the namespace-macro change.)

The counter-force, measured rather than assumed

Every slice re-parses the same template headers, so total elapsed could have risen. reg_flow.cppis that measurement — it parses impl/cuda/reg_flow.h plus the kernels and instantiates nothing, at 0.20 GiB and 1.64 s. Twelve extra TUs of re-parsing is therefore ~20 s against a 1069 s baseline, under 2%, an order of magnitude below the instantiation work that now divides.

It is a ratio, not a law: a module whose per-TU instantiation work is comparable to ~1.6 s should not be split.

What this does not buy: a higher -j

The ceiling is max(peak) over every module. reg_field (8.35 GiB) and reg_field_rls (7.07 GiB) are untouched, so reg_flow stops being the constraint and reg_field becomes it — and their 15.42 GiB overlap against a 16 GB runner is the same -j2 hazard the Makefile already documents, neither improved nor worsened here. Do not raise -j on the strength of this change. Splitting reg_field and reg_field_rls the same way — structurally identical files, the port is a prefix rename of this diff — is what would take max(peak) low enough to make -j4 CPU-bound rather than RAM-bound.

Gate

src/lib-cuda/ is the entire diff — sixteen files, none outside that directory — and nothing on the CPU side compiles or includes any of them: src/lib-cpu and src/lib build their own sources against include/, which this branch does not touch. So the 13-suite / 59,886-check gate cannot move, and CI's path filter skips those legs on this PR for exactly that reason — they show as skipped, not green, which is worth not misreading.

Because CI skips them, I ran the gate here by hand: tools/test-baseline.sh --legs default,lib on this branch, diffed row for row against tools/test-baseline.expected15 rows, 0 differ; 13 suites / 59,886 checks / 0 failures on the default leg, plus the 2 hub suites (14 checks). (--check itself refuses a two-leg run, since it compares whole reports and the recording covers six legs, so the rows were diffed against the recording directly.)

Both build-cuda legs are green, including the --no-undefined hub link, tools/check-cuda-host-stubs.sh and the ldd -r load-time resolution check; compile-probe-cuda is green.

No GPU in CI, so nothing here is a claim about runtime behaviour — the CUDA side is compile + link only.

lint (clang-format, changed lines) is red, as it is on every PR; per #89 it cannot print its findings, so it is not chased here. #156 has since made it informational.

reg_flow is the binding constraint on this whole project's CI. nvcc peaks at
12.98 GiB compiling it -- 81% of a 16 GB runner, in one indivisible TU -- and
that single number is what pins `make cuda` at -j2, which in turn is what
makes a multi-architecture -gencode set unaffordable against the 120-minute
timeout.
The cost is template instantiation, and it factors. Each exported entry point
selects exactly one internal wrapper template (_flow_matvec, _flow_matvec_acc,
_flow_diag, _flow_kernel, _flow_relax) and dispatches it over
ndim x boundary x dtype x offset width. No two arms of that pyramid share an
instantiation, so a TU that only ever reaches `_flow_diag<2, ...>` pays for
`_flow_diag<2, ...>` and nothing else. Cutting the file along those seams cuts
the instantiation set the same way.
What is here
------------
* reg_flow.cpp keeps all 15 exported entry points, every argument check and
every error message, and instantiates NOTHING. It normalises strides,
validates, decides the offset width, and `switch (ndim)`es into a slice.
205 MB / 2.0 s to compile.
* reg_flow_slice.h -- the seam: one hidden-visibility declaration per
(entry point, ndim), 30 in all.
* reg_flow_slice.inl -- the wrapper templates and the dtype x offset x bound
dispatch, verbatim from the single-TU form, plus the slice definitions the
including TU selects with FF_FLOW_SLICE_*.
* reg_flow_{matvec,diag,kernel,relax}_{1,2,3}d.cpp -- twelve four-line TUs.
Why a forwarding seam and not `-D` on one source
------------------------------------------------
Compiling reg_flow.cpp N times with different -D flags, the way BOUNDFLAGS
already varies a build, gives N objects that each define ff::cuda::flow_matvec
and the link fails. The exported entry point has to live in exactly one TU.
`extern template` was the other candidate and was rejected: it has to name
every leaf, and with FF_STATIC_BOUNDS=0 six of the eight FF_BOUND_* selectors
collapse onto bound::type::Dynamic, so the enumeration would contain the same
specialization six times -- ill-formed under [temp.explicit]/5, and which
leaves collapse is a build-flag decision. A `switch` has no such problem:
duplicate *implicit* instantiations across its arms are the same
instantiation.
ABI
---
The slice symbols are __attribute__((visibility("hidden"))), so
libfastfields-cuda.so's dynamic symbol table is unchanged. Every slice
function is defined by exactly one TU in MODULES -- too few and the link fails
undefined, too many and it fails duplicate. The Makefile list and the
FF_FLOW_SLICE_* selection cannot silently disagree.
Also: reg_flow.cpp now includes impl/kernels/utils.h explicitly. FF_CANUSE32BITS
expands to an unqualified canUse32BitIndexMath, which is declared there and NOT
in core/autocast.h, whose include comment in core/dispatch.h says otherwise.
Every other dispatch source pulls the kernels in wholesale and never noticed;
the front TU is the first that does not.
Measured numbers land in the Makefile once build-cuda has run on this branch.
…ow files
Pre-emptive, so that neither sweep has to touch these files and neither
conflicts with them:
* #145 -- one include-guard convention, no `#pragma once`. reg_flow_slice.h
dropped the pragma and its guard is spelled the way that PR derives new
ones from the path: FF_SRC_LIB_CUDA_REG_FLOW_SLICE_H.
* #146 -- the public interface is spelled <fastfields/...>. Same-directory
siblings stay quoted, which is what that PR does to impl/cuda/reg_flow.h's
`#include "utils.h"`.
Include spelling only; the preprocessed output is unchanged. Front TU still
compiles and every slice still preprocesses.
Numbers from `build-cuda` on this branch (run 32365351378) against the same
job on 85fdac7 (run 32325044770) -- same runner image, same nvcc 12.0.140,
same -O1, same bound/spline policy.
heaviest TU 12.98 -> 1.95 GiB (-85.0%)
module CPU 1149.89 -> 751.95 s (-34.6%)
wall -j2 2243 -> 1672 s (-25.5%, whole `make cuda`)
.so link 1.38 -> 1.05 s
The header-parse counter-force is recorded because it is the reason this
could have gone the other way: a TU that parses the whole template header set
and instantiates nothing costs 0.22 GiB and 1.35 s, so twelve extra TUs is
~16 s against a 1149.89 s baseline. Measured, not assumed -- and stated as a
ratio so the next module can be judged on its own numbers rather than on
this one's result.
The section is blunt about the limit: this does NOT raise the -j ceiling.
max(peak) is now reg_field at 8.23 GiB and the reg_field/reg_field_rls
overlap is unchanged at 15.31 GiB, so -j2 stays the ceiling and stays luck.
A scheduling model over the measured table reproduces the observed -j2 wall
to 0.25%, and says -j4 would still peak at 19.26 GiB on a 16 GB box.
@balbasty

Copy link
Copy Markdown
CollaboratorAuthor

ok but should use pragma once and the namespace macros

claudeand others added 3 commits August 20, 2026 16:26
Brings in #143 (per-backend FF_INDEX32), #145 (#pragma once), #146
(<fastfields/...> includes) and #151 (the measured index-axis table).
One conflict, in src/lib-cuda/reg_flow.cpp: this branch splits the file and
moves the dtype x offset dispatch macros into reg_flow_slice.inl, while #143
rewrote the narrow arm of those same macros from int32_t to off32_t and #146
rewrote the include lines. Resolved by taking this branch's structure with both
of main's changes applied in their new home:
* all 20 narrow dispatch arms in reg_flow_slice.inl now name off32_t, which
is exactly the count main's reg_flow.cpp carries;
* the new files already spelled public includes <fastfields/...> and kept
the same-directory sibling quoted, so #146 needed nothing;
* reg_flow_slice.h switched from an #ifndef guard to #pragma once -- #145
landed the opposite way round from its original proposal, and the new
files followed the proposal rather than the merge.
tools/normalise-header-guards.py --check, tools/normalise-include-delimiters.py
--check and tools/rename-macros.py --check are all clean on the result.
Re-verified after the merge:
* tools/test-baseline.sh --legs default,lib -> row-for-row identical to
tools/test-baseline.expected across all 15 rows. 59,886 checks / 13 suites
for the default leg, plus the 2 hub suites, 0 failures.
* every one of the 30 slice functions is still defined exactly once across
the twelve slice TUs (preprocessor census: 13 occurrences each -- twelve
declarations from the shared header plus one definition).
* the front TU still instantiates nothing: 205,400 kB, 1.8 s.
* the heaviest slice still compiles: 2,037,652 kB, matching the 2,039,660 kB
build-cuda reported for it before the rebase (0.1%).
#151 recorded what FF_INDEX32 costs on the unsplit modules. This is the same
question asked of a slice, because the interesting case is whether splitting
makes the axis cheap enough that the knob stops mattering.
It does not. Same TU, index axis toggled:
reg_flow_matvec_3d FF_INDEX32=1 1.94 GiB 277.1 s 17.0 MB
FF_INDEX32=0 1.05 GiB 174.4 s 10.0 MB
-45.8% peak, which is the same proportion #94 measured on the unsplit file with
the host compiler (-44.6%). The split and the knob multiply rather than
overlap: 12.98 -> 1.94 GiB from one, 1.94 -> 1.05 GiB from the other.
build-cuda's index64 leg builds this split with the index axis off, and that
combination -- not either half alone -- is the first configuration in which no
module in this directory is heavy any more. Measured on that leg:
reg_field 3.76 GiB 264.90 s <-- the new maximum
reg_field_rls 3.42 GiB 279.38 s
resize 1.08 GiB 90.20 s
reg_flow_matvec_3d 1.05 GiB 81.45 s <-- heaviest reg_flow slice
reg_flow (front) 0.20 GiB 0.88 s
Total nvcc CPU 1380.3 s; `make cuda -j2` wall 723 s, measured. Feeding that
table through the scheduling model (which reproduces the measured -j2 wall to
0.5%) puts -j4 at a 393 s makespan with a worst coincidence of 9.31 GiB --
comfortably inside a 16 GB runner, with reg_field untouched.
Recorded with the caveat that matters: this holds on the index64 leg, not on
index32, which is the default and the one that ships. There the same model
still says 19.26 GiB at -j4. The note says so explicitly so nobody reads the
index64 number as licence to raise -j globally.
@balbastyClaude

Copy link
Copy Markdown
CollaboratorAuthor

CI note, so the state is legible to whoever picks this up.

The run on the final commit (3366779, run 32394048973) is stalled in Actions
infrastructure, not on anything in this branch: compile-probe-cuda, which
takes ~21 s, has been in_progress for over 30 minutes, and the previous run
(32392090091) had its build-cuda (index32) leg sit past the 120-minute timeout
while the same commit'sindex64 leg finished normally in 12 minutes. Worth
re-running rather than reading as a result.

Between them, the two earlier runs already cover both legs of the matrix:

runcommitlegresult
323653513788f08c7dbuild-cuda (pre-#143, single leg = index32)success, 1672 s compile
323920900917b20ab9build-cuda (index64)success, 723 s compile

8f08c7d and 7b20ab9 carry an identical split; the difference between them is
the rebase onto b0463bc (off32_t, #pragma once, <fastfields/…>), which
changes no instantiation. So the split is known-good at index32, and the rebased
tree is known-good at index64 — the two together cover the matrix, and
3366779 on top of 7b20ab9 is a Makefile-comment-only commit.

Both FFMEM tables are transcribed into src/lib-cuda/Makefile and the PR body.

lint (clang-format, changed lines) is red and is continue-on-error; per #89
it cannot print its findings, so I have not chased it.


Generated by Claude Code

@balbastyClaude

Copy link
Copy Markdown
CollaboratorAuthor

Correcting my previous comment.

I said the run on 3366779 was "stalled in Actions infrastructure". That was
wrong, or at least unsupported, and I would rather fix it than leave it standing.

What actually happened is that the Actions API was serving me a cached view of
run 32394048973 — its updated_at sat at 16:49:03 across half an hour of
polling while the individual step timestamps did advance underneath. The two
steps I cited as evidence both completed normally once the cache caught up:

  • compile-probe-cuda — 16:49:02 → 16:50:54, 112 s, success;
  • Install CUDA toolkit and clang on the index64 leg — 16:49:04 → 16:51:03,
    119 s, success.

So there is no reason to think anything is wrong with this run; it was simply
still compiling, and I was reading a stale snapshot as if it were a live one.

The one observation that does still stand is separate and from the earlier
run: 32392090091's build-cuda (index32) leg had a real step start of
16:29:20 and had not completed by 18:27, past the job's own 120-minute
timeout, while the same commit's index64 leg finished in 12 minutes. That
remains anomalous against the 27.9 minutes the identical split took at index32
in run 32365351378, and is worth a re-run rather than a diagnosis from me.

Net effect on the PR: none of the measurements change — they come from
32365351378 (index32) and 32392090091 (index64), both of which completed and
both of whose FFMEM tables are transcribed into src/lib-cuda/Makefile. The
only thing I got wrong was the status report, not the data.


Generated by Claude Code

The seam header spelled `namespace ff { namespace cuda { namespace
flow_slice {` by hand -- the one file in the split that did. CLAUDE.md is
explicit that the namespace is opened with FF_NAMESPACE_BEGIN(FF_NS) /
(FF_DEVICE) / (<module>) and never hard-coded, so that a header compiled for
either backend lands in the right one rather than asserting `cuda`.
Pulls in core/cuda_switch.h, which is where FF_DEVICE comes from and which
already provides FF_NAMESPACE_BEGIN via core/defines.h.
The other new files (reg_flow_slice.inl and the twelve slice TUs) already
used the macros; this makes the set consistent.
#155 fixed nvcc writing `::exit(1)` into the host object in place of every
impl/kernels/utils.h helper, which is the state the first measurement of this
split was taken in. Both sides of the table are now post-#155 runs of the
same job, so nothing quoted here comes from a library that could not run.
The finding is that #155 did not move the compile at all: unsplit reg_flow
peaks at 12.98 GiB before and after the fix, because the code #155 restored
is host code and the peak belongs to a device-side process. The split's case
is unchanged, but it is now stated from runs that measure a working library.
Re-measured, index32 (BEFORE run 32397332659 / AFTER run 32418602148):
heaviest reg_flow TU 12.98 -> 1.94 GiB (-85.0%)
reg_flow elapsed, sum 1069.30 -> 803.32 s (-24.9%)
`make cuda -j2` wall 2145 -> 1861 s (-13.2%)
max peak, any module 12.98 (reg_flow) -> 8.35 (reg_field)
and the index64 leg besides, where the elapsed saving is much smaller (-5.7%)
because collapsing the offset axis had already removed half the
instantiations -- while memory still falls by four fifths.
Two corrections to how the previous numbers were framed. The elapsed and
wall-clock savings are smaller than first reported (-24.9% and -13.2%, not
-34.6% and -25.5%); reg_field, which nothing here touches, moved 848.87 ->
861.05 s between the same two runs, so wall-clock on this job carries several
percent of noise and is quoted with that caveat. Peak RSS does not: all
thirteen per-slice peaks reproduce the earlier run to within 0.01 GiB, and
reproduce off-runner on a different machine to 0.1-0.7%.
Also annotates the #80 table above, whose reg_flow row no longer describes a
translation unit that exists, with post-#155 figures for the modules this
change does not touch.
Sign up for freeto 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.

2 participants

@balbasty@claude