Uh oh!
There was an error while loading. Please reload this page.
build: make the 32-bit index axis a per-backend option (FF_INDEX32), default unchanged - #143
Merged
Merged
Conversation
Every templated kernel below the dispatch layer is templated on `offset_t`, which has exactly two values chosen per call by `canUse32BitIndexMath`: `int32_t` when every operand's largest element offset fits in 32 bits, `int64_t` otherwise. That axis costs exactly x2 instantiations of everything underneath -- on CUDA, x2 device code, x2 SASS and x2 ptxas memory. Measured on reg_flow, the module that peaks at 12.98 GB of a 16 GB runner: dropping it is -50.3% instantiations and -44.6% peak compiler RSS (fastfields-lib#94). Nobody has ever benchmarked what the narrow path buys here -- it is an ATen inheritance and the argument for it (register pressure) is a GPU argument, while there is no GPU in CI. So this does not change the default. It makes the axis a build-time policy, per backend, so the position is reversible once somebody with hardware can answer the question. FF_INDEX32=1 (default, both backends) today's behaviour, unchanged FF_INDEX32=0 the narrow arm names int64_t too, so both arms are one instantiation, the axis collapses, and the canUse32BitIndexMath probe folds away Spelling follows BOUNDFLAGS/SPLINEFLAGS exactly: ONE macro, with the default chosen separately by each library's own Makefile via a new INDEXFLAGS variable. That per-library default IS the per-backend mechanism -- FF_STATIC_BOUNDS is one macro too, and only differs between backends because the two Makefiles say so. An FF_INDEX32_CPU/FF_INDEX32_CUDA pair would instead force core/dispatch.h to branch on __CUDACC__, putting the policy in the source in the one directory that is backend-agnostic by contract, and every third consumer would have to know both names. INDEXFLAGS stays outside CXXFLAGS for the same reason the other two do: CUDA CI overrides CXXFLAGS to force -O1 and must not silently drop a policy while doing it. The type the narrow arm names is a typedef, `ff::off32_t`, not a macro -- CLAUDE.md's own guidance (a name in ff:: is collision-safe with no prefix at all), and it keeps the 140-line source change a pure token swap of the same width, so not one line-continuation backslash moves. CI gains the two legs that make this an option rather than a claim: a `test-cpu (clang-index64)` leg that runs the whole suite with the axis off, and `build-cuda` as a two-leg matrix over the two positions -- the off leg is the cheaper of the two and turns the per-module FFMEM table into a direct measurement of what the axis costs on the backend where it costs the most.
This was referenced Aug 20, 2026
Open
Uh oh!
There was an error while loading. Please reload this page.
balbasty pushed a commit
that referenced
this pull request
Aug 20, 2026
Resolved by taking main's tree wholesale for include/, src/, tests/ and CLAUDE.md, then re-running the sweep script over the merged base -- the procedure the committed script exists for. #146 (the <fastfields/...> delimiter sweep) and #143 (the per-backend FF_INDEX32 option) both landed underneath this branch and both touch the header preambles this sweep rewrites.
This was referenced Aug 20, 2026
balbasty pushed a commit
that referenced
this pull request
Aug 20, 2026
#146 (angle-bracket includes) and #143 (FF_INDEX32) landed on main while this proposal was being written, which turned the script's central claim from an assertion into something demonstrable. The rebase was done the way the docstring prescribes -- revert, merge main, re-run unedited -- and the same script emitted <fastfields/...> on the new base with no edit and no hand-merging. Both runs are now recorded in Appendix A. Also corrects the vox migration cost: 26 FF_NAMESPACE_BEGIN(<module>) sites across 25 files, not 28 across 26, with the per-module breakdown. The earlier figure came from a scan that counted non-module namespaces (bound, spline, meta, internal, _splinc) alongside the module ones. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
balbasty pushed a commit
that referenced
this pull request
Aug 20, 2026
… on the new base Regenerated by re-running `tools/move-core-headers.py` unedited on de288a9 (main after #146 and #143), which is the whole point: the earlier prototype commit was reverted rather than hand-merged, and this is what the script produced on its own. 10 headers moved, 4,262 lines 81 files rewritten, now emitting <fastfields/...> instead of "fastfields/..." `--check`: clean, idempotent, no dependency leak Drop this commit and the design in the preceding ones still stands. The one-line tools/test-baseline.sh probe update rides along here because the script does not touch that file: the gate tool probes for impl/kernels/bounds.h to validate tree layout and dies before running a single test if it is absent. tools/consolidate.sh and the six-repo probe in the same script name the same paths and are deliberately left alone -- they describe the frozen pre-consolidation layout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
This was referenced Aug 20, 2026
balbasty pushed a commit
that referenced
this pull request
Aug 20, 2026
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%).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the 32-bit index axis a build option, per backend, defaulting to
today's behaviour on both. Nothing about the shipping libraries changes; what
changes is that the axis can be turned off for one backend without touching the
other, and that both positions are now built by CI.
Follow-up to the measurement in #94 (draft, not merged). It takes that PR's
FF_INDEX32mechanism only — not the dispatch-driver experiment, and not thestruct-wrapping that re-indents whole files.
The axis, and what it costs
Every templated kernel below the dispatch layer is templated on
offset_t,which has exactly two values chosen per call by
canUse32BitIndexMath:int32_twhen every operand's largest element offset fits in 32 bits,int64_totherwise. So the narrow path is exactly ×2 instantiations ofeverything underneath — on CUDA, ×2 device code, ×2 SASS, ×2 ptxas memory.
core/autocast.hexists solely to feed that second instantiation.The spelling, and why
FF_INDEX32— one macro incore/dispatch.h— plusINDEXFLAGS, one makevariable defaulted separately in
src/lib-cpu/Makefileandsrc/lib-cuda/Makefile.That is the
BOUNDFLAGS/SPLINEFLAGSpattern verbatim, and the per-librarydefault is the per-backend mechanism:
FF_STATIC_BOUNDSis also one macro,and only differs between backends because the two Makefiles say so. An
FF_INDEX32_CPU/FF_INDEX32_CUDApair would instead forcecore/dispatch.hto branch on__CUDACC__— putting the policy in the source,in the one directory that is backend-agnostic by contract — and any third
consumer would have to know both names.
INDEXFLAGSalso stays outsideCXXFLAGS, for the reason the other two do: CUDA CI overridesCXXFLAGStoforce
-O1and must not silently drop a policy while doing it.The type the narrow arm names is a typedef,
ff::off32_t, not a macro —CLAUDE.md's own guidance ("prefer an
inlinefunction to a macro where onewill do; a name in
ff::is collision-safe without any prefix"). It is alsoexactly as wide as
int32_t, so the 140-line source change is a pure tokenswap and not one line-continuation backslash moves.
Default unmoved — object-level proof
All ten
src/lib-cpumodules compiled frommain(85fdac7) and from thisbranch at the default produce byte-identical objects:
(
clang++ -std=c++11 -O1 -fPIC -DFF_STATIC_BOUNDS=0 -DFF_STATIC_SPLINES=0,cmpon the.o.)Gate, locally,
make test-lib-cpu: 13 suites, 59,886 checks, 0 failures, inboth positions. In CI the four pre-existing
test-cpulegs,sanitizeandtsanare all green, andtest-cpu (clang-index64)'s per-suite counts sumrow-for-row to
tools/test-baseline.expected(2352 + 4622 + 704 + 4012 + 308 +6381 + 19250 + 16347 + 186 + 630 + 65 + 452 + 4577 = 59,886).
tools/rename-macros.py --check→0 file(s) would change,include/ is clean.codespellclean.Both positions build and pass
CI gains the two legs that make this an option rather than a claim:
test-cpu (clang-index64)— the whole suite withFF_INDEX32=0. Green.build-cudabecomes a two-leg matrix,index32(shipping, the gate) andindex64. Both green, both through the full compile + hub link +ldd -rcheck. The off leg is the cheaper of the two (~19 min vs ~38), andit turns the
FFMEMtable into a direct measurement of the axis on thebackend where it costs the most.
Numbers
CPU —
clang++ -O3 -fPIC,BOUNDFLAGS/SPLINEFLAGSat the CUDA shippingpolicy. "instantiations" = defined weak symbols in the object.
reg_flowmainreg_flowFF_INDEX32=0reg_fieldmainreg_fieldFF_INDEX32=0pushpull_backwardmainpushpull_backwardFF_INDEX32=0posdefmainposdefFF_INDEX32=0Reproduces #94's table (it measured 18,952 KB / 1,126 MB for
reg_flowonmainand 9,661 KB / 624 MB with the axis off).Whole
libfastfields-cpu.so, all-Dynamic policy:FF_INDEX32=1FF_INDEX32=0The off-position symbol set is a strict subset: 13,440 symbols disappear,
0 appear, and every one that disappears is an
offset_t = int32_ttemplateinstantiation. No public entry point changes — the exported ABI is the same
library with half the template bodies.
CUDA — both legs of one
build-cudarun, nvcc-O1 -j2, shippingbound/spline policy. Peak nvcc RSS (GiB) / wall per module:
FF_INDEX32=1FF_INDEX32=0reg_flowreg_fieldreg_field_rlsresizereg_flow_rlspushpull_backwardpushpullrestrictdistancesplincposdefCUDA is worse than the host-compiler extrapolation implied, not better:
reg_flowsheds 56.1% rather than the 44.6% clang showed. This table is nowrecorded above
MODULESinsrc/lib-cuda/Makefile.Two things worth flagging from the left-hand column, since the existing note
there claims the heavy modules reproduce to better than 0.1%:
reg_flowdoes(12.97 vs the recorded 12.98 GiB),
reg_fielddoes not (8.09 vs a recorded8.93). Recorded rather than silently overwritten.
What the stated preference costs — plainly
The decision on #94 is to keep 32-bit dispatch, with a per-backend option
because "it's probably more important on CUDA than on CPU". The
register-pressure instinct is very likely right. The consequence is worth
seeing in one place, without argument:
reg_flowpeaks at12.97 GiB of a 16 GB runner;
build-cudagates atFF_MEM_BUDGET_KB= 14 GiB(ci(cuda): gate nvcc peak memory on a budget, and record why -j2 is not headroom #95); under
-j2reg_field+reg_flowpeaks sum to over the limit andthe build is green only because the two spikes have not yet coincided.
register file to protect.
is not constrained and leaves the constrained one exactly where it is.
After this PR the CUDA build still sits at ~13 GiB against a 16 GB runner,
and anything that makes a regulariser heavier —
-arch/-gencodeflags,half/bf16, a new energy term — spends from that same ~1 GiB of margin.
That is not an argument against the decision. It is the reason the knob is
worth having: the position is now reversible by one make variable, and the
numbers for the other position are on the record instead of being guessed at.
The benchmark that would settle it — scoped, not run
The narrow path is an ATen inheritance and, as far as anyone can tell, has
never been benchmarked in this repository. There is no GPU in CI and no nvcc
on the machine this was prepared on, so it cannot be settled here. The spec is
#144: which ops, which shapes, which
ncumetrics, and a decision rulefixed in advance so the result is not re-litigated afterwards. An afternoon's
work once hardware exists.
One related cost, noted and not chased: on CUDA the narrow path is paid for
with a
cudaMallocHost+cudaFreeHostper array per call(
core/autocast.h), justified in that header as enabling an async H2D copy —but 363 of the 391 upload sites under
include/fastfields/impl/cuda/callthe synchronous
copyToDevice(onlydistance_{euclidean,l1,mesh}.husecopyToDeviceAsync), so for almost every op there is no async copy for thepinning to enable. If that cost is real it comes off whatever the register win
is worth, worst exactly where the kernel is cheapest. #144 says to measure it
alongside and report it separately.
Cost of this PR
The
build-cudamatrix roughly doubles that job's runner-minutes onCUDA-triggering changes — but the second leg is the cheaper one (~19 min of the
~57 total), the two run in parallel so wall clock is unchanged, and it is the
only thing that makes the off position a supported configuration rather than an
untested claim.
lint (clang-format, changed lines)is red; it iscontinue-on-errorandcannot report its own findings (#89), and the source diff here is a
same-width token swap that moves no formatting.