Skip to content
This repository was archived by the owner on Aug 20, 2026. It is now read-only.

[teeny] Phase B2: posdef entry points take teeny carriers — delete the _any shared-size reconstruction - #67

Merged
balbasty merged 1 commit into
teenyfrom
claude/62-posdef-carrier-boundary
Aug 5, 2026
Merged

[teeny] Phase B2: posdef entry points take teeny carriers — delete the _any shared-size reconstruction#67
balbasty merged 1 commit into
teenyfrom
claude/62-posdef-carrier-boundary

Conversation

@balbasty

@balbastybalbasty commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Agent:claude-fastfields-to-teeny

Phase B2 of the tensor-native-boundaries umbrella (fastfields-lib#57) — the impl half.

This is one logical change split across two repos only because GitHub cannot span them: the cpu-lib half is fastfields/fastfields-cpu-lib#80, and neither PR compiles without the other (posdef.cpp is the only caller of these entry points, and it converts in the same PR-set). Please review them together — same split convention as Phase A's cpu-impl#60 / cpu-lib#74.

Convention reference: fastfields-lib/TEENY-MIGRATION.md §9.

Closes#62


What changed

All eight entry points took a decayed (nbatch, nchannel, T*, size[], stride_*[]) tuple and immediately reassembled the tensors their caller had just taken apart. They now take the carriers themselves:

template <type Ty, wr W, int C, typenamereduce_t, classAO, classAH, classAI>
staticvoid_matvec(AO ao, constAH ah, constAI ai);
template <type Ty, typenamereduce_t, classAO, classAI, classAH>
voidsolve(AO ao, constAI ai, constAH ah); // no weighttemplate <type Ty, typenamereduce_t, classAO, classAI, classAH, classAW>
voidsolve(AO ao, constAI ai, constAH ah, constAW aw); // with weight

nbatch, nchannel, size[] and every stride_*[] argument are gone (R2/R3). With them goes the _any helper this file used to open with — the textbook R3 hazard §9.2 cites: one shared size[] for every operand, each carrier rebuilt from the shared batch dims plus a per-tensor trailing extent (nchannel for the vectors, C(C+1)/2 for the packed hessian), because a single shared array could otherwise decode one operand against another's geometry. Per-tensor carriers retire that class structurally. Its per-call std::vector heap allocation goes with it.

kernels/posdef/matrix.h is not touched at all (load-bearing for the gate) — it was already tier-1 tensor-native. The per-thread Cholesky workspace and the peel_front_at<-1> voxel loop are unchanged; only the prologue moved out. Read-only operands are const-element carriers (R4). Old signatures are deleted, not deprecated.


The gate — all criteria, with evidence

Measured on one machine, clang++ 18.1.3 and g++ 13.3.0, -std=c++17 -DTNY_MAX_RANK=64.

Measurement hygiene. This container is shared with two other migration agents, so every number below comes from dedicated git worktrees (base = teeny tip, new = base + this PR-set) wired to a private kernels worktree. diff -rq over the two trees reports exactly the three changed files and nothing else; the base side is git status-clean at cpu-impl@21833c4 / cpu-lib@0b03229. An earlier pass measured against the shared checkouts was discarded and retaken rather than reported.

Method / anti-fooling

objdump -s gives raw section bytes per symbol (every template instantiation is its own .text._Z... comdat), so "byte-identical" means exactly that, not "the disassembly looks alike". Separately, objdump -d is parsed per symbol with branch/call targets and %rip-relative displacements blanked — they encode placement and relocations, not content — while every other operand is compared literally.

Symbols are paired by demangled name after stripping element-type constness only (read-only operands become carriers of const T, so a kernel that only reads its operand gets a new mangled name for identical code). Nothing else is normalised — in particular extents<int,…> is not mapped onto extents<long,…>, so the disappearing int32 instantiations show up as unmatched rather than being quietly paired with their int64 twins.

Per-element loops are located structurally, by opcode: within each symbol every backward branch whose target is inside that symbol closes a loop, and the innermost such bodies (those containing no nested backward branch) are the per-element work. So a loop is still found after the compiler moves it, renames its enclosing symbol, or re-inlines it somewhere else.

Negative control: the same harness run on base -O2 vs base -O3 correctly reports DIFFERS (325 differing symbols, 1124 new-only loop bodies), and base -O2 vs itself reports all-identical. The "identical" verdicts below are therefore not an artefact of over-normalisation.

1. Kernel-layer instantiations byte-identical at -O2 — ✅ (one explained exception at g++ -O3)

kernels/posdef/matrix.h is untouched by this PR-set. Every one of its instantiations that exists as a standalone symbol:

buildkernel symbols (base → new)byte-identicaldiffering
clang++ -O236 → 1414 / 140
clang++ -O331 → 1111 / 110
g++ -O2133 → 5151 / 510
g++ -O363 → 3534 / 351 (below)

On clang, every base symbol without a partner is an extents<int,…> instantiation — i.e. exactly the int32 arms D1/R5 deletes, 22 at -O2 and 20 at -O3, with zero non-int32 casualties.

The one g++ -O3 exception, stated plainly.sym::invert_w_<double> is not byte-identical. It is 820 → 820 instructions with a zero mnemonic-histogram delta, and the entire difference is one adjacent pair swapped:

 call TARGET
mov 0x8(%rsp),%rax
+ movdqa RIP,%xmm5
movapd RIP,%xmm4
- movdqa RIP,%xmm5
mov (%rax),%rdi

Two independent constant loads into different registers, reordered by the scheduler. No instruction added, removed or changed — this is g++ whole-TU scheduling jitter in a function whose source did not change (the Phase A nuance), not a code change. Its float twin is byte-identical.

2. Driver loops: no new instructions in the per-element loop — ✅

Per entry-point family, the multiset of innermost loop bodies (exact operands):

familyclang++ -O2clang++ -O3g++ -O2g++ -O3
matvec/addmatvec_/submatvec_0 new-only0 new-onlymigration ¹migration ¹
sym_matvec_backward0 new-only0 new-onlymigration ¹0 new-only
sym_invert/sym_invert_0 new-only0 new-onlymigration ¹migration ¹
kernels (matrix.h)0 new-only0 new-only0 new-only0 new-only
solve/solve_arity split ²arity split ²arity split ²arity split ²

On clang, at both optimisation levels, the matvec / backward / invert per-element loops and every kernel loop are literally a subset of the base's — nothing was added anywhere in them. The control instantiation _matvec<Sym,set,-1,double> is 377 → 377 instructions with an identical body-size multiset [19,19,20,23,27,27,27,28,28,28].

¹ The g++ migration, stated plainly. On g++ the drivers shrank (the _any prologue left), so g++'s inliner now pulls the tiny Eye/Diag/ESTATICS micro-kernels into the driver instead of calling them. Their loop bodies therefore move from the "kernels" bucket to the "driver" bucket, and a per-symbol identity claim is simply the wrong question there. Measured instead on _matvec<Eye, add, C=2>, whole-function (the kernel is fully unrolled at static C, so a loop-only count would miss its arithmetic in the base build):

base driverbase eye::matvec calleenew drivernew callee
elem=float390 insns26 insns184 insnsnone (inlined)
elem=double386 insns16 insns175 insnsnone (inlined)

The per-element floating-point op multiset is identical on both sides ({addsd:2, cvtsd2ss:2, cvtss2sd:5, movss:2, mulsd:2} for float; {addsd:2, movsd:4, mulsd:2} for double), and the totals fall 416 → 184 and 402 → 175 — the same arithmetic, one call/return per element fewer, minus the deleted _any prologue. No new kernel symbol appears in the new build on any compiler.

² The solve/solve_ arity split, stated plainly. This is the one per-element loop that genuinely changed shape, and it is a reduction. The base driver carried a runtime if (have_w) inside the voxel loop with both dispatch arms inlined; the weight is now an arity, so each driver contains one arm and no test. The base's single 48-instruction voxel body has no counterpart in the new build:

solve family, per-element body sizes (clang++ -O2)
base : … (27, 62) (28, 70) (48, 44) largest = 48
new : … (27,118) (28,120) (45, 10) largest = 45

Per instantiation, solve<Sym,double>: base bodies [4,11,14,27,27,27,27,28,28,28,28,48] → new [14,14,27,27,27,27,27,27,27,28,28,28,28,28,28,28]; solve_<Sym,double>: base [4,11,27,27,27,28,28,28,48] → new [27,27,27,27,27,28,28,28,28,28]. The 48-instruction body is gone from both; the body count doubles only because one merged driver became two specialised ones. Total instructions per instantiation rise (718 → 911 for solve) purely as code size across two specialisations — and the file-level instantiation count still halves (1478 → 776 symbols at clang -O2) because the int32 arms are gone.

The remaining new-only bodies are all per-call prologue, not per-element: the lib entry points' own CHECK_SAME_BATCH loops and tny::_dl::import_anyrank's metadata copy — which is not new work either, just relocated: the base built the same copy_meta carrier one layer down inside _any, plus a std::vector heap allocation per call that is now gone.

3. Oracle unchanged — ✅

With the pre-existingtests/test_posdef.cpp, both trees give exactly the issues' stated baseline:

basenew
test_posdef, original test file5092 / 05092 / 0

Identical on clang++ and g++, in both trees — four runs, all four reporting test_posdef5092 / 0:

runtreecompilertest filetest_posdef
Abaseclang++pre-existing5092 / 0
Bbaseg++pre-existing5092 / 0
Cnewclang++pre-existing5092 / 0
Dnewg++pre-existing5092 / 0

diff of the full 12-binary check listing, base vs new, is empty on both compilers with the pre-existing test file.

The +876: what it is, and why it is not drift

test_posdef gains run_descriptor_variants, covering the two DLPack descriptor features this module used to normalise by hand and which had no coverage at all on any posdef entry point:

  • strides == NULL (DLPack's compact row-major shorthand — previously ContiguousStrides),
  • byte_offset != 0 (previously VOIDPTR/CVOIDPTR), and both together

— three modes × two dtypes, each exercising all eight exported entry points against the same brute-force oracle, plus pad sentinels asserting that the elements in front ofbyte_offset are never written (on the read-only operands too, so a mis-folded offset on an input fails loudly). That is 146 checks × 2 dtypes × 3 modes = 876, exactly the delta.

Crucially, those 876 checks also pass, with the identical 5968 total, against the pre-refactor code — verified by compiling the new test file against the untouched base worktree. They pin existing ABI behaviour; they do not encode the new implementation.

4. ASan / UBSan — ✅

-fsanitize=address,undefined -fno-omit-frame-pointer, ASAN_OPTIONS=detect_leaks=1:halt_on_error=1, UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1, on the touched path (posdef.cpp + tests/test_posdef.cpp): 5968 checks, 0 failures, no sanitizer reports, exit 0. Built with g++ — this box has no clang libclang_rt.asan, the same limitation Phase A recorded.

5. Whole suite green, unchanged counts on every other module — ✅

All 12 test binaries × 8 runs (base/new × clang++/g++ × pre-existing/shipped test file) = 96 binaries, every one PASSED, 0 failures.

binarybase (orig tests)new (orig tests)new (shipped tests)
test_distance306730673067
test_distance_mesh462246224622
test_distance_spline704704704
test_posdef509250925968 +876
test_pushpull326326326
test_reg_dispatch603060306030
test_reg_field525525525
test_reg_flow112311231123
test_reg_op186186186
test_resize413041304130
test_restrict191191191
test_splinc457745774577

Identical table on clang++ and g++ (diff of the two compilers' listings is empty). diff of base vs new is empty for the pre-existing test file, and empty again for the shipped test file — the only line that ever moves is test_posdef, and it moves by the deliberate +876 on both trees.

Verified cleans (the cpu-lib#56 trap). A genuine make clean ran at every compiler/tree switch and the script recorded the post-clean file count each time; all four reads were post-clean: objects/libs/deps=0 test-binaries=0. The four +876 runs are incremental rebuilds of test_posdef alone (no other test binary depends on tests/test_posdef.cpp), and are labelled as such rather than claimed as clean builds. All 8 runs exited MAKE_EXIT=0 with no Terminated/Killed.


Judgment calls

1. Template shape: one parameter per TENSOR, and the element type is no longer one of them.
Phase A (#60) established "one template parameter per tensor, plus the ordinary deduced parameter per scalar", and kept scalar_t because dt() had a scalar argument (w) that had to name a type. None of posdef's eight entry points has a scalar argument left, so scalar_t and offset_t both disappear from the impl signatures and the carriers carry them. This does not move a dispatch decision down a layer (R1):*-lib still chooses from_dlpack<float> vs <double> exactly where it used to choose scalar_t, and reduce_t, the layout Ty, the static C and the write mode W all still arrive from there. offset_t is derived one line in (using offset_t = decltype(ao.size(0))), which is the carrier's own type — geometry, so R2 applies.

2. The element-type static_assert compares carriers to each other, not to a named type.
With no scalar_t parameter there is nothing external to check against, so the invariant becomes "all operands of one call agree", via a small _same_elem<...> fold over _elem_t (which strips cv, since read-only operands are const-element carriers). A mismatch is one named message instead of a pointer-conversion cascade inside matrix.h.

3. The optional weight is an ARITY, and one variadic core serves both.
The issue asked for two solve overloads rather than a null-data carrier, and that is what the public surface is. To avoid writing the same twelve-line driver four times (solve × solve_ × with/without weight), each pair forwards to one internal core taking the weight as a pack of size 0 or 1 that expands straight into the dispatch call. The public spelling is still exactly the two overloads the issue specifies, and static_assert(sizeof...(AW) <= 1) keeps the pack honest. This is also what removes the runtime have_w test from the voxel loop (gate 2, note ²).

4. The runtime _packed<Ty,C>(nchannel) is deleted; _packed_s<Ty,C>() is kept verbatim.
The issue says "CC for validation/recast still comes from _packed/_packed_s (kept — the C > 0 static recast fast path is untouched)". After the prologue moves out, the compile-time twin is still needed (the static-C recast needs CC as a constant expression) but the runtime one has no caller left in this file at all: its only callers were the _any calls that computed the hessian carrier's trailing extent, and *-lib now hands that carrier down already shaped by the DLPack descriptor. I read the issue's parenthetical as the binding part (don't lose the static recast path) and deleted the genuinely dead helper rather than leave it. fastfields-cuda-impl/posdef.h has its own copy and is out of scope, so nothing else breaks. Flagging it explicitly in case the intent was to keep both.

5. sym_invert/sym_invert_ derive C from CC rather than taking nchannel.
These are the only two entry points where the channel count is not any operand's extent — both operands are packed, so the carriers answer CC, not C, and C is needed only to size the C×C Cholesky workspace. Strictly, R2 ("no parameter where a carrier already answers the question") would permit passing nchannel here. I followed the issue's explicit "nbatch, nchannel, size[] and every stride_*[] argument disappear" instead, adding a four-line _channels_sym(CC) inverse. It is exact on every value that reaches it because *-lib validates CC as a triangular number (channels_from_packed) and throws first, and it runs once per call, outside the voxel loop. The alternative — keeping one scalar argument on two of eight entry points — is defensible too; I went with the issue's wording and am recording the trade-off rather than burying it.

6. parallel_for lambdas say int64_t, not long.
Phase A's judgment call 3: parallel_for is declared (int64_t, int64_t, int64_t, const F&) and invokes f(begin, end) with int64_t, but the lambdas declared long. Same type on LP64 (hence the byte-identical object code), 32-bit on LLP64/Windows. Phase A asked that Phase B apply the same spelling wherever it touches one; all six rewritten lambdas here do.

7. Not touched, deliberately: TEENY-MIGRATION.md.
§7's impl paragraph still describes the _any helper this PR-set deletes, and §9.4's phase table still shows Phase A as . Phase A's PRs did not edit that document either (cpu-impl#60 explicitly recorded the same choice and filed its §9 R2 finding as a note instead), so I have kept to the two-repo scope the issues define. §7 and §9.4 both need a follow-up edit in fastfields-lib — happy to open it.


Generated by Claude Code

…econstruction
Phase B2 of the tensor-native-boundaries umbrella (fastfields-lib#57), the
cpu-impl half of one coordinated change with fastfields-cpu-lib#76 (which
imports the carriers via tny::from_dlpack). Convention: TEENY-MIGRATION.md
sec. 9.
All eight entry points -- matvec / addmatvec_ / submatvec_, sym_matvec_backward,
solve, solve_, sym_invert, sym_invert_ -- took a decayed
(nbatch, nchannel, T*, size[], stride_*[]) tuple and immediately reassembled
the tensors their caller had just taken apart. They now take one anyrank
carrier per operand, built lib-side from that operand's OWN DLPack descriptor,
so nbatch, nchannel, size[] and every stride_*[] argument disappear (R2/R3).
That retires the `_any` helper this file opened with -- the textbook R3 hazard
sec. 9.2 cites: ONE shared size[] for every operand, each carrier rebuilt from
the shared batch dims plus a per-tensor trailing extent, because the vectors
trail C while the packed hessian trails C(C+1)/2. Per-tensor carriers retire
that class structurally, and the helper's per-call std::vector heap allocation
goes with it.
kernels/posdef/matrix.h is NOT touched: it was already tier-1 tensor-native.
The per-thread Cholesky workspace and the peel_front_at<-1> voxel loop are
unchanged; only the prologue moved out. Read-only operands are const-element
carriers (R4), so writing through a peeled cell of one is a compile error.
The optional weight becomes an ARITY -- two solve/solve_ overloads -- not a
carrier over a null pointer, so the pointer-era sentinel stops at the DLPack
boundary. One variadic core serves both arities, which also removes the runtime
have_w test from the voxel loop.
The runtime _packed<Ty,C>(nchannel) helper is deleted (its only callers were
the _any calls); the compile-time _packed_s<Ty,C>() twin the static-C recast
needs is kept verbatim. sym_invert[_] derive C from the packed length via a
four-line _channels_sym inverse, exact because *-lib validates CC first.
Gate: matrix.h instantiations byte-identical at -O2 wherever they exist as
standalone symbols (clang 14/14 and 11/11; g++ 51/51; one g++ -O3 symbol
differs by a two-instruction scheduler swap at equal instruction count and zero
mnemonic delta). Per-element loops: zero new-only loop bodies on clang for the
matvec/backward/invert families and every kernel; solve/solve_'s body shrinks
from 48 instructions to <=28 as the have_w test leaves the loop. test_posdef
5092 -> 5968 checks (+876 deliberate, all passing on the pre-refactor code
too), 0 failures, clang++ and g++, whole 12-binary suite unchanged elsewhere,
asan+ubsan clean.
Closes#62
Claude-Session: https://claude.ai/code/session_015xcQBMEdA5eQtTsx2wB3AC
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@balbastybalbasty added enhancement New feature or request claude-fastfields-to-teeny labels Aug 4, 2026 — with Claude
@balbasty
balbasty merged commit 47ca535 into teenyAug 5, 2026
1 check failed
@balbasty
balbasty deleted the claude/62-posdef-carrier-boundary branch August 5, 2026 06:11
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@balbasty@claude