Skip to content

Regulariser dispatch: one per-surface header instead of six copies - #158

Merged
balbasty merged 2 commits into
mainfrom
feat/reg-dispatch-header
Aug 20, 2026
Merged

Regulariser dispatch: one per-surface header instead of six copies#158
balbasty merged 2 commits into
mainfrom
feat/reg-dispatch-header

Conversation

@balbasty

@balbastybalbasty commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Answers points 3 and 5 of the review on #94. Replaces that PR's
include/fastfields/api/dispatch.h, which was correctly called out as
mis-named: its content was tied to reg_field's template order, and
core/dispatch.h already owns the name for the genuinely surface-independent
helpers.

What this is worth on its own

Readability, and nothing else — that is the whole claim. It is measured
instantiation-neutral (below), so it makes no memory argument and should not be
reviewed as if it did. What it buys is that the regulariser dispatch pyramid is
stated once instead of six times, and its innermost fan twice instead of
fifty-two times.

The measurement that motivated it

Counting the dtype × index-width fan — the innermost 2×2 every dispatch site
ends in — by where the copies live on main:

fansoff32_t mentions
the 6 regulariser TUs52104
the other 15 TUs, combined1936
whole tree71140

73% of the duplication is in this one surface, because it copies the fan once
per entry point
rather than once per module — thirteen times in
reg_field.cpp alone. Every other surface has one to three copies in total,
which is why this is a per-surface header and not a general dispatcher; the
reasoning is written out in the #94 reply.

The rest of the pyramid is near-exact across the six regulariser TUs
(src/lib-cpu/reg_{field,flow}.cpp, src/lib-cuda/reg_{field,flow}{,_rls}.cpp):

  • BND1 / BND2 / BND3byte-identical in all six.
  • BOUND_SWITCHbyte-identical in all six.
  • NDIM_SWITCH — differs in exactly one word, the noun in its diagnostic:
    "Only 1D, 2D and 3D field are supported" versus "… flow …". That word
    is now the NOUN parameter.

What the header does

include/fastfields/api/cpu/reg_dispatch.h, the same shape and location as the
existing api/{cpu,cuda}/pushpull_dispatch.h:

  • FF_REG_BND1/2/3 — the boundary pack, unchanged text.
  • FF_REG_DT / FF_REG_DT_OP — the dtype × index fan, twice. Two and not
    one because the surface genuinely has two leaf shapes. Of the thirteen
    dispatch sites in each file, eight thread a compile-time char op as the
    leaf's second template argument (addmatvec_, submatvec_, and diag /
    kernel in all three variants) and five have no op concept at all
    (plain matvec, relax, and the three _rls ops). Both macros take OP so
    the levels above can forward one argument list to either; FF_REG_DT ignores
    it.
  • FF_REG_BOUND, FF_REG_NDIM — once each.
  • FF_DISPATCH_REG(FN, NOUN, …) / FF_DISPATCH_REG_OP(FN, OP, NOUN, …) — the
    two call forms.

Call sites go from a #define …_ARGS / NDIM_SWITCH(…_DT) / #undef triple to
one call with an ordinary argument list:

FF_DISPATCH_REG(_field_matvec, "field",
bvec, static_cast<int64_t>(nbatch), nc, FF_VOIDPTR(out), FF_CVOIDPTR(inp),
voxel_size, absolute, membrane, bending,
out.shape, out.strides, inp.strides)

26 sites converted, 13 per file. Net −473 lines across the two TUs against
+176 for the header.

Behaviour is unchanged, and this is checkable at the object level

The FF_BOUND_<NAME> selectors, the switch labels, the boundary-pack lengths
and every diagnostic string are the same tokens they were; only their location
moved. The BOUNDFLAGS static/dynamic routing is untouched.

clang++ -std=c++11 -O3 -fPIC with the CUDA shipping bound policy
(-DFF_STATIC_BOUNDS=0 -DFF_STATIC_BOUND_DCT2=1 -DFF_STATIC_BOUND_DST2=1),
counting defined text/weak symbols:

TUinstantiationsobject (bytes)
reg_field.cppmain10,14019,533,368
this PR10,14019,533,360
reg_flow.cppmain10,70419,407,392
this PR10,70419,407,392

reg_flow's object is byte-identical. reg_field has identical symbol
sets
(all 22,909, name for name) and identical text/data/bss — the
eight-byte file difference is non-loadable metadata. Details in the comments
below.

Instantiation-neutrality is the constraint that matters here: the leaf set is
what ptxas has to process once the CUDA mirror follows.

I am deliberately not quoting compile time or peak RSS. That run had three
builds sharing four cores and the timings were not reproducible (reg_flow
measured 216 s on main and 1,929 s here — an artefact, not a regression).
Instantiation count and object size are deterministic under contention; time is
not.

Variadic macro syntax

Argument lists use ISO ... / __VA_ARGS__ rather than the GNU named-variadic
args... the other dispatch macros still use. This is a portability down
payment, not an MSVC fix: MSVC's traditional preprocessor also mis-forwards
__VA_ARGS__ into a nested macro (it arrives as one argument), so a nested
pyramid would need /Zc:preprocessor regardless. Nothing in this tree builds
under MSVC and I have not tested any of that — there is no MSVC here.

Scope: CPU only, on purpose

The four CUDA regulariser TUs want the mirror header and it is a mechanical
follow-up. It is not in this PR because src/lib-cuda/reg_flow.cpp is being
rewritten by #147's per-(family, ndim) TU split, and #147 has independently
grown its own copy of this pyramid (src/lib-cuda/reg_flow_slice.inl, with
FF_FLOW_BND1/2/3, FF_FLOW_MV_DT, …). Those two should be reconciled once
#147 lands, rather than conflicting now.

Because this touches only api/cpu/ and src/lib-cpu/, CI's path filter
correctly skips the CUDA legs.

Gate

Local: 13 suites, 59,886 checks, 0 failures, plus the 2 hub suites, row for
row identical to tools/test-baseline.expected — full rows in a comment below,
including a note on why --check exits 1 on a two-leg run even when every row
matches.

CI: all test-cpu legs, both lint jobs and codespell green.

Related: #160 records something found while measuring for this PR —
resize/restrict/splinc are outside the BOUNDFLAGS policy entirely.
Independent of this change.

The regulariser API surface -- `reg_field` (multi-channel fields) and
`reg_flow` (vector flows) -- carried its ndim x bound x dtype x index-width
dispatch pyramid as per-file macros. Measured across the six regulariser
translation units on main:
* BND1 / BND2 / BND3 and BOUND_SWITCH are byte-identical in all six.
* NDIM_SWITCH differs in exactly one word: the noun in its diagnostic,
"field" versus "flow".
* The dtype x index-width fan is copied once per ENTRY POINT rather than
once per module -- thirteen times in reg_field.cpp alone, fifty-two
across the six, out of seventy-one in the whole tree. Each copy differs
only in the leaf's name and, for nine of them, one extra `char`
template argument.
This moves that pyramid into include/fastfields/api/cpu/reg_dispatch.h, the
same shape and location as the existing api/{cpu,cuda}/pushpull_dispatch.h,
and converts the two CPU regulariser translation units to it. The fan is now
written twice (once per leaf shape: with and without the compile-time op
char) instead of fifty-two times; the boundary and rank switches once each.
Behaviour is unchanged by construction. The FF_BOUND_<NAME> selectors, the
switch labels, the boundary-pack lengths and every diagnostic string are the
same tokens they were; only their location moved.
Instantiation-neutral, which is the constraint that matters here -- the leaf
set is what ptxas has to process on the CUDA side. clang++ -O3 -fPIC with the
CUDA shipping bound policy, counting defined text/weak symbols:
reg_field main 10,140 this 10,140 (identical; object 19,533,368 ->
19,533,360 bytes)
Argument lists use ISO `...` / `__VA_ARGS__` rather than the GNU named
variadic `args...` the other dispatch macros still use. That is a
portability down payment and not a fix: MSVC's traditional preprocessor
also mis-forwards __VA_ARGS__ into a nested macro, so an MSVC build would
need /Zc:preprocessor regardless. Nothing here builds under MSVC and that
is untested.
Scope is deliberately the CPU side only. The four CUDA regulariser TUs want
the mirror header, but src/lib-cuda/reg_flow.cpp is being rewritten by the
per-(family, ndim) TU split, so that half should follow rather than collide
with it.
C++11. Clean under clang++ and g++ at -std=c++11.
@balbastyClaude

Copy link
Copy Markdown
CollaboratorAuthor

The reg_flow half of the instantiation measurement has finished, and it is a
stronger result than the reg_field one — the object is byte-identical, not
merely the same size class.

clang++ -std=c++11 -O3 -fPIC, CUDA shipping bound policy
(-DFF_STATIC_BOUNDS=0 -DFF_STATIC_BOUND_DCT2=1 -DFF_STATIC_BOUND_DST2=1),
counting defined text/weak symbols ([TWt] in nm -C):

TUinstantiationsobject (bytes)
src/lib-cpu/reg_field.cppmain10,14019,533,368
this PR10,14019,533,360
src/lib-cpu/reg_flow.cppmain10,70419,407,392
this PR10,70419,407,392

So the leaf set — the thing ptxas has to process once the CUDA mirror follows —
does not move at all, and on reg_flow neither does a single byte of the
object. The eight-byte difference on reg_field is the only delta anywhere in
the two objects.

Restating the caveat from the description, because this run made it vivid: the
wall-clock column from the same run is not usable. reg_flow measured
216 s on main and 1,929 s on this branch — an artefact of three builds
sharing four cores, not a 9× regression. Instantiation count and object size
are deterministic under contention; time and peak RSS are not, which is why the
claim rests only on the first two.


Generated by Claude Code

@balbastyClaude

Copy link
Copy Markdown
CollaboratorAuthor

Chasing the eight-byte object-size difference on reg_field to ground, since it
was the only delta anywhere and "nearly identical" is a weaker claim than this
change can actually make.

It is not code. Same build as above:

$ nm -C main-reg_field.o | awk '{$1="";print}' | sort > main.syms
$ nm -C new-reg_field.o | awk '{$1="";print}' | sort > new.syms
$ diff main.syms new.syms # no output
$ wc -l < main.syms; wc -l < new.syms
22909
22909
$ size main-reg_field.o
text data bss dec hex filename
9526354 26656 36 9553046 91c496 main-reg_field.o
$ size new-reg_field.o
text data bss dec hex filename
9526354 26656 36 9553046 91c496 new-reg_field.o

Identical symbol sets — all 22,909, name for name — and identical text,
data and bss to the byte.
The eight bytes are in non-loadable metadata
(section/string table), not in anything that is linked or executed.

Combined with reg_flow's object being byte-identical outright, that is as
close to a proof of behavioural equivalence as an object-level check gets:
every leaf that main instantiates, this branch instantiates, with the same
mangled name, and the emitted code is the same size in every section. It is
also the check I would want on the CUDA mirror when that follows, since ptxas
memory is a direct function of exactly this set.

(Method note for anyone repeating it: tools/macro-equivalence.py is the right
instrument for the other kind of change — proving a hoisted FF_* macro
expands token-for-token to the local one it replaced. It is keyed to
core/dispatch.h's macro names and parameter shapes, so it does not apply
here. Comparing the objects is the equivalent question for this change, and its
own docstring explains why whole-TU -E diffs are the wrong tool for either.)


Generated by Claude Code

@balbastyClaude

Copy link
Copy Markdown
CollaboratorAuthor

Local gate, as promised in the description.

$ tools/test-baseline.sh --tree . --legs default,lib
distance default 2352 0
distance_mesh default 4622 0
distance_spline default 704 0
posdef default 4012 0
pushpull default 308 0
pushpull_backward default 6381 0
reg_field default 19250 0
reg_flow default 16347 0
reg_op default 186 0
resize default 630 0
restrict default 65 0
solve_field default 452 0
splinc default 4577 0
lib_device_check lib 5 0
lib_splinc_bound lib 9 0

13 suites, 59,886 checks, 0 failures, plus the 2 hub suites (14 checks) —
and row for row identical to the default and lib rows of
tools/test-baseline.expected:

$ diff <(grep -v '^#' tools/test-baseline.expected | awk -F'\t' 'NF>=4 && ($2=="default"||$2=="lib")' | sort) \
<(measured rows | sort)
# no output

Note for anyone re-running it: --legs default,lib --check tools/test-baseline.expected
exits 1 even when every row matches, and it is not a test failure —

BASELINE NOT COMPARABLE against tools/test-baseline.expected
recorded configs: cuda-default default dynamic lib sanitize static
measured configs: default lib
--check compares whole reports, so re-run with the same --legs the recording used.

--check compares the whole report, and test-baseline.expected was recorded
with --legs all (six configs). So a two-leg run can never satisfy --check as
written; the comparison above is the same check done against the subset of rows
that the two legs actually cover. Worth knowing before someone reads that exit
code as red. Not something this PR should change.

CI's test-cpu matrix is the stronger version of this anyway — it runs the
same suite across five bound/spline/index policies plus ASan/UBSan and TSan,
where this is one policy.


Generated by Claude Code

The header said nine dispatch sites thread a compile-time `char` op and seven
do not. Counted properly it is eight and five, of thirteen per file:
with op addmatvec_, submatvec_, diag, adddiag_, subdiag_,
kernel, addkernel_, subkernel_
without op matvec, relax, matvec_rls, diag_rls, relax_rls
Comment only; no code change.
@balbasty
balbasty merged commit d3320b4 into mainAug 20, 2026
15 checks passed
@balbasty
balbasty deleted the feat/reg-dispatch-header branch August 20, 2026 22:47
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