Uh oh!
There was an error while loading. Please reload this page.
fix(kernels): utils.h helpers must be host+device, not device-only (#150) - #155
Merged
Conversation
`impl/kernels/utils.h` declared its whole first half -- swap, square, sqrt,
pow, min, max, abs, sign, mod, typed_prod, prod, fillfrom, fill -- as
`inline FF_CUDEV`, i.e. `__device__` only. Host code calls them:
* `canUse32BitIndexMath`, in this same header, calls `typed_prod`. That is
the edge behind every `FF_CANUSE32BITS` in `src/lib-cuda`.
* Every `FF_CUHOST` launcher in `impl/cuda/{reg_field,reg_flow,
distance_euclidean,distance_l1,distance_mesh}.h` calls `prod(size, n)` on
its first line, to size the grid.
* `impl/kernels/distance/mesh.h`'s `FF_CUHOST build_tree` calls `max`.
nvcc rejects a host->device call only when the calling function is not itself
a template. All of the above are templates, so nvcc emitted no diagnostic and
cudafe++ wrote `{int volatile ___ = 1; ...; ::exit(___);}` into the HOST
object in place of each callee's body. The result links cleanly, passes
`-Wl,--no-undefined` and `ldd -r` (the damage is intra-TU), terminates the
process with status 1 on the first call, and -- `exit` being `noreturn` --
loses every statement after that call at -O1 and above.
Make the whole header `FF_CUHOSTDEV`. Nothing in it is device-specific;
device codegen is unchanged and the host side gains one inline function per
used instantiation. Without nvcc both macros expand to nothing, so the CPU
layers preprocess byte-for-byte identically (verified over all ten
`src/lib-cpu/*.cpp`).
Two gates, because the fix alone leaves the trap set:
* `tests/impl-cuda/compile_probe_hostdev.cu` calls each helper from a
plain, non-template `__host__` function -- the shape nvcc does check --
so a re-qualified helper is a compile error again. It produces 22 errors
against the pre-fix header and compiles in ~2 s. It also explicitly
instantiates one launcher per affected header, which type-checks the
launcher body in the device pass where the call edge is reported.
* `tools/check-cuda-host-stubs.sh`, run in `build-cuda`, fails on any
undefined `exit` in `build/obj/lib-cuda/*.o`. Nothing here calls
`::exit`, so that symbol has exactly one source.
`compile_probe_mesh.cu` documented this exact nvcc error as "a spurious
artifact of the probe technique, not a defect". It was neither; that comment
is corrected in place.
Fixes#150.Uh oh!
There was an error while loading. Please reload this page.
balbasty pushed a commit
that referenced
this pull request
Aug 20, 2026
#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.
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.
Fixes#150. The report is right about the defect and wrong about three of its
details — one of them the part that decides whether the shipped library works.
Everything below is measured on nvcc 12.0.140, the version
build-cudainstalls.
The mechanism — what nvcc actually does
nvcc does reject a
__host__→__device__call. But only when thecalling function is not itself a template. That is the whole hazard, and it is
not documented as a limitation anywhere in the CUDA guide.
__host__function__device__function__host__function__device__function template__host__function template__device__function__host__function template__device__function templateRows 1–2 give
Rows 3–4 give
rc=0and empty stderr.--Werror all-warningsand-Xcudafe --display_error_numberadd nothing: there is no diagnostic topromote.
In the silent rows
cudafe++writes this into the host object in place ofthe callee's real body — the real one is kept beside it under
#if 0:Everything follows from that. It is a call to
exit, not an error, andexitis
noreturn, so from-O1up the host compiler deletes every statement afterthe call. The buggy entry point at
-O1is five instructions:Correction 1 —
-O0is not the safe end of the rangeThe issue reads
refs: 3at-O0as "not affected". It is affectedidentically; only the object-code symptom is optimisation-dependent. Linked
and run (no GPU needed — nothing is launched):
-O0-O1prodf1 reached/returned normally, exit 0__device__-onlyprodAt
-O0the dispatch is still in the object and still never reached: theprocess is gone before it. This is a process-terminating defect at every
optimisation level, not dead code at
-O1+. For a library loaded into a Pythonprocess, that is
exit(1)under the user's interpreter.Correction 2 —
FF_INDEX32=0is not a workaround, andindex64is not correctThe follow-up comment concludes that "
index64produces a functionally correctlibrary and
index32… does not". That holds forsplincand does notgeneralise.
canUse32BitIndexMathis only one of three host callers:canUse32BitIndexMath→typed_prod, behind everyFF_CANUSE32BITS;FF_CUHOSTlauncher inimpl/cuda/{reg_field,reg_flow, distance_euclidean,distance_l1,distance_mesh}.h→prod(size, n)on itsfirst line, to size the grid;
impl/kernels/distance/mesh.h'sFF_CUHOST build_tree→max.Only the first is behind
FF_INDEX32. Measured on unmodifiedb0463bc,distance.cpp,-O1,FF_INDEX32=0:Exactly the three whose launchers live in the three headers that call
prodfrom host code. The spline entry points, whose launchers do not, are fine.
Turning the axis off moves the truncation from the entry point down into the
launcher; it does not remove it. Neither
build-cudaleg produced a workinglibrary.
CI agrees: in the
index64leg, the only module whose peak RSS moves on thisbranch is
distance, +15.2% (564,184 → 650,204 kB) — the work that wasbeing discarded there, now being done.
Correction 3 — the blast radius is 100%, and "78 sites" is not the unit
"78" counts
FF_CANUSE32BITSlines; there are 123 occurrences insrc/lib-cuda. Neither is the blast radius, because one entry point can containseveral and because the launcher edge is not counted at all. The unit that
matters is exported entry points whose body is truncated, measured per
object by walking each object's call graph to the
exitstub:ff::cuda::entry pointsexit(1)onmainEvery entry point of the public CUDA API. Not "every dispatch" — the whole
surface.
And the compiler had already said so
tests/impl-cuda/compile_probe_mesh.cuhas carried this since the mesh port:It was not an artifact. That is #150, reported correctly by nvcc and dismissed;
an explicit instantiation was the one thing in the tree that made the check
fire. Corrected in place, and the technique is now used deliberately.
The fix
impl/kernels/utils.h's entire first half —swap,square,sqrt,pow,min,max,abs,sign,mod,typed_prod×2,prod×2,fillfrom×4,fill×2 — wasinline FF_CUDEV. All 28 becomeFF_CUHOSTDEV. Nothing in theheader is device-specific, and its second half (
StaticValueand its 60-oddoperators) was already
FF_CUHOSTDEV, so the header is now uniform. Devicecodegen is unchanged; the host side gains one inline function per used
instantiation.
How the class was found, three ways that agree: (a)
nm -u | grep -w exitoverevery built object — an undefined
exithas exactly one source here, sincenothing in the tree calls
::exit; (b) grep for unqualified calls to eachutils.hname fromFF_CUHOSTcontext, which is what turned up the launchersand
build_tree; (c)FF_CUHOSTappears 129 times ininclude/, few enough toread.
The guards — worth more than the fix
tests/impl-cuda/compile_probe_hostdev.cu— calls every helper from aplain, non-template
__host__function, i.e. the shape nvcc does check.Against the pre-fix header it produces 22 errors; against this branch it
compiles in 2.5 s. It also explicitly instantiates one launcher per
affected header, which re-checks the whole launcher body in the device pass.
Picked up automatically by
compile-probe-cuda(PROBESRCis a wildcard).tools/check-cuda-host-stubs.sh— newbuild-cudastep overbuild/obj/lib-cuda/*.o, fails on any undefinedexit. Runs in 2 s and isthe catch-all for edges the probe does not enumerate.
Nothing that exists today could have gone red: the damage is intra-TU, so
--no-undefinedandldd -rboth pass; front-end instantiation is unaffected,so the
FFMEMbudget does not move; and the CPU suite cannot see it becauseboth macros are empty without nvcc.
Cost — measured, and the memory gate does not trip
build-cuda (index32), this branch vs run 32388984092 (mainatb0463bc),same runner image, same flags:
.solinkCompile step wall: 34m24s → 35m45s (+3.9%).
reg_flowstays at 12.98 GiBagainst the 14 GiB
FF_MEM_BUDGET_KB, with the same ~1.02 GiB of headroom.Why the peaks barely move, and why that is not evidence the fix does nothing.
FFMEMisru_maxrss— a maximum over the nvcc process tree. Restoring thedeleted code adds work to the host pass (
cc1plus). For the small modulesthe host pass is the peak, so it shows: distance +23.5%, posdef +11.2%, the
.solink +23.3%. For the regularisers the peak iscicc/ptxasin thedevice pass, which this change does not touch at all, so a real increase in
host work is invisible to that metric — it surfaces instead in compile time
(reg_field +8.6%, pushpull +12.1%, restrict +15.5%, pushpull_backward +17.7%)
and in object size (locally, at identical flags: distance 7.20 → 12.30 MB,
resize 25.14 → 29.79 MB).
The uncomfortable corollary:
FF_MEM_BUDGET_KBcould never have caught thisdefect and cannot confirm its repair. It measures the pass that was always
working. Worth noting next to the #147 discussion — the split is still the right
call, but not because of anything here.
The same across all 11 modules on a local rig (all-Dynamic policy,
-O1,FF_INDEX32=1, so absolute values differ from CI; the ratios are the point):peaks move ≤0.03% for resize/restrict/splinc/reg_field/reg_field_rls/reg_flow/
reg_flow_rls, and +20% to +52% for distance/posdef/pushpull/pushpull_backward —
exactly the modules whose peak is the host pass.
CPU path — unchanged, proved not assumed
FF_CUDEVandFF_CUHOSTDEVboth expand to nothing outside nvcc, so all tensrc/lib-cpu/*.cpppreprocess byte-for-byte identically before vs after(
g++ -E -P, ten of ten). And the gate itself:(
--checkinsists on--legs all, so the comparison is done by diffing thedefault/librows, as #147 also did.)Green
build-cuda (index32),build-cuda (index64),compile-probe-cuda, all sixtest-cpulegs,test-hub,codespell. Both new gates ran and passed.lint (clang-format)iscontinue-on-errorand unreadable (#89) — not chased.No GPU in CI
Nothing here is a runtime claim. What still needs real hardware: that the
restored dispatch selects the right instantiation for each dtype/dim/bound, that
the launchers launch and the kernels compute correctly, the CUDA
streamplumbing, the atomics, and the mesh
sdtpath. This PR establishes only thatthe code is present and reachable — which it was not, anywhere, for any entry
point.
Adjacent
#154 also edits
.github/workflows/ci.ymlandCLAUDE.md; the two touchdifferent regions but will want ordering. #147 is gated on this fix and is
unaffected by it in its own measurements.