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

[teeny] Phase A: dt_l1/dt_euclidean import via tny::from_dlpack - #74

Merged
balbasty merged 2 commits into
teenyfrom
claude/72-distance-from-dlpack
Aug 4, 2026
Merged

[teeny] Phase A: dt_l1/dt_euclidean import via tny::from_dlpack#74
balbasty merged 2 commits into
teenyfrom
claude/72-distance-from-dlpack

Conversation

@balbasty

Copy link
Copy Markdown
Contributor

Agent:claude-fastfields-to-teeny

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

This is one logical change split across two repos only because GitHub cannot span them: the impl half is fastfields/fastfields-cpu-impl#60 (which converts dt() to take the carrier), and neither PR compiles without the other. Please review them together — same split convention as phase 6's cpu-lib#70 / cuda-lib#39.

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

Closes#72


What changed

dt_l1/dt_euclidean held a complete tensor and disassembled it by hand to feed the impl's raw-args signature. tny::from_dlpack does all three parts by construction — folds byte_offset into the pointer (was VOIDPTR), expands a NULLstrides field to row-major (was ContiguousStrides), and copies the shape/stride metadata into the carrier — so the carrier is built once, in the existing dtype dispatch, and handed straight down:

template <typenamescalar_t = float>
inlinevoid_dt_l1(DLTensor & inp_out, double w)
{
auto at = tny::from_dlpack<scalar_t>(&inp_out);
distance_l1::dt(at, static_cast<scalar_t>(w));
}

ContiguousStridesstays in autocast.h — the spline/mesh paths in this same file still use it (5 remaining uses; Phases D/E, last-user rule), and autocast.h itself is untouched. The l1/euclidean paths already had no int32 arm, so there was no narrowing to delete (R5 effectively pre-applied by the distance-slice review).


Preserved behaviour — each item checked, not assumed

1. Exported signatures byte-for-byte unchanged.distance.h untouched; the mangled symbols are identical before/after:

base: T _ZN2ff3cpu12dt_euclideanER8DLTensordi T _ZN2ff3cpu5dt_l1ER8DLTensordi
new: T _ZN2ff3cpu12dt_euclideanER8DLTensordi T _ZN2ff3cpu5dt_l1ER8DLTensordi

2. Error messages and their evaluation order.DISPATCH_DT's body, its float/double arms and its exact rejection string ("only floating point data types are supported") are untouched — the shims keep scalar_tleading their parameter list precisely so the macro's func<float> spelling did not have to change. CHECK_NO_LANES still runs first, on the caller's own descriptor, so the order is unchanged: lanes check → dtype switch throw, and the throw is still reached before any import. from_dlpack's own dtype_matches is a debug _TNY_CHECK that the user-facing throw pre-empts; test_bad_dtype_throws (float16) covers exactly this and still passes.

3. strides == NULL still works — now via from_dlpack's expansion, and now actually tested (it never was; see the +715 below).

4. byte_offset != 0 still works — was VOIDPTR, now from_dlpack, with a direct test plus a sentinel proving nothing before the offset is written.

5. DLPack include order (R7).<teeny/dlpack.h> is included after this repo's vendored "dlpack.h", which was already first in the block — the TU stays on fastfields' DLPack v1.2 throughout (teeny vendors v1.1; same guard, first one wins). I confirmed this was already the natural order and did not reorder it; a comment now records why the order matters.


The Phase A 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. A verifiedmake clean ran before each compiler — the script records the post-clean file count each time, and all four reads were post-clean: objects/libs/deps=0 test-binaries=0 (the #56 trap: a green run that silently reused the other compiler's binaries).

1. kernel instantiations byte-identical at -O2 — ✅

Compared as raw section bytes (objdump -s), not merely disassembly. The mangled names are unchanged (the kernel signature did not change), so this is a like-for-like comparison of the same symbol:

symbolclang++ -O2g++ -O2clang++ -O3g++ -O3
distance_e::kernel<long, float>byte-identicalbyte-identicalbyte-identicalinlined ¹
distance_e::kernel<long, double>byte-identicalbyte-identicalbyte-identicalinlined ¹

¹ At -O3 g++ inlines the euclidean kernel into the per-line lambda, so it emits no standalone section — and that enclosing symbol (…dt<…>::{lambda(long,long)#1}::operator(), 340/346 instructions) is itself identical before/after, so the kernel's code is still proven unchanged, just as part of a larger identical unit.

distance_l1::kernelnever emits a standalone symbol on either compiler — it is small enough to be fully inlined into the driver. For l1, criterion 1 is therefore subsumed by criterion 2, and the per-line-loop result below is the kernel result.

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

The l1 per-line work is a 7-instruction min-sweep body (add / min / inc / store / advance / cmp / jne), emitted once per sweep direction per unrolled copy. Locating every such body by opcode — wherever the compiler chose to put it — and comparing:

builddtypebase sweep-body sizesnew sweep-body sizesverdict
clang++ -O2float[6,6,6,6,9,9,9,9,9,9,19,19]sameidentical
clang++ -O2double[6,6,7,7,8,8,9,9,10,10,19,19]sameidentical
clang++ -O3float24 bodiessameidentical
clang++ -O3double24 bodiessameidentical
g++ -O2float[7,7,7,7][7,7,7,7]identical
g++ -O2double[7,7,7,7][7,7,7,7]identical
g++ -O3float[7,7,7,7,7,7][7,7,7,7,7,7]same count + same mnemonics; operand-only ²
g++ -O3double[7,7,7,7,7,7][7,7,7,7,7,7]same count + same mnemonics; operand-only ²

Instruction count and mnemonic sequence are identical in all eight combinations. Nothing was added anywhere, so there is nothing to explain away or eliminate.

Whole-symbol comparison, matched by structural role (names legitimately change — dt's signature changed):

  • clang++, -O2 and -O3: every distance driver symbol is instruction-for-instruction identical — parallel_for<lambda> (serial branch, 253–338 insns), __invoke_impl (195–275), lambda::operator() (184/190), _M_invoke, _M_manager. 18 symbols, all identical, both opt levels.
  • g++ -O2: euclidean lambda::operator() (147) identical; l1 parallel branch _M_invoke (89) identical; l1 serial branch — 32-instruction window, mnemonics identical, exactly one operand differs: movss 0xc(%rsp)movss 0x1c(%rsp), i.e. the spill slot of w moved 16 bytes because the enclosing stack frame no longer builds the carrier locally. Same instruction, different displacement.
  • g++ -O3: euclidean lambda::operator() (340/346) identical; l1 parallel branch (105) identical; l1 serial branch same body count and mnemonics, differing only in register allocation and operand order (cmp %r13,%rsicmp %rsi,%r13).

² The g++ -O3 serial-branch differences are register-allocation and operand-order choices, not extra work — g++ re-inlines the (now smaller) function differently, and the enclosing function actually shrinks (221 → 184 instructions). The inner sweep is the same 7 instructions.

Method / anti-fooling:objdump -d parsed per symbol; branch and call displacements blanked (they encode placement, not content) while every other operand is compared literally. Negative control: the identical harness run on base -O2 vs base -O3 correctly reports DIFFERS with a diff, so the "identical" verdicts are not an artifact of over-normalisation.

3. Oracle unchanged — ✅

test_distance was 2352 checks / 0 failures at baseline on both compilers (the issues say "2,350"; the true figure is 2352 — 2350 numeric comparisons plus 2 bad-dtype-throw checks). After the refactor, with the original test file, it is still exactly 2352 / 0 on both compilers.

The committed suite then adds 715 deliberate checks (§ below), so the shipped figure is 3067 / 0 on both compilers.

ASan + UBSan (-fsanitize=address,undefined, detect_leaks=1, halt_on_error=1, print_stacktrace=1) on distance.cpp + test_distance.cpp: 3067 checks, 0 failures, no sanitizer reports, exit 0. Built with g++ — this box has no clang libclang_rt.asan, so the clang sanitizer build could not be run here.

4. Whole suite green with unchanged counts — ✅

All 12 test binaries (the issues say 11; there are 12), both compilers, genuine clean between. diff of the full baseline vs post-change check listing yields exactly one differing line on each compilertest_distance:

binarybaselineafter
test_distance23523067+715 deliberate (see below)
test_distance_mesh46224622unchanged
test_distance_spline704704unchanged
test_posdef50925092unchanged
test_pushpull326326unchanged
test_reg_dispatch60306030unchanged
test_reg_field525525unchanged
test_reg_flow11231123unchanged
test_reg_op186186unchanged
test_resize41304130unchanged
test_restrict191191unchanged
test_splinc45774577unchanged

Identical table on clang++ and g++; every binary PASSED. distance.cpp also serves the untouched spline/mesh paths, and test_distance_mesh / test_distance_spline are unchanged — those paths did not move.

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

test_distance gains test_descriptor_variants, covering the two DLPack descriptor features this path used to normalise by hand:

  • 4 cases with strides == NULL (DLPack's compact-row-major shorthand — previously ContiguousStrides), both transforms × both dtypes, each against the brute-force oracle. MIGRATION.md lists strides==NULL as a historical soft spot and no test exercised it at all.
  • 4 cases with byte_offset != 0 (previously VOIDPTR), plus a sentinel assertion that the 5 padding elements in front ofbyte_offset are not written — so a mis-folded offset fails loudly instead of reading right and writing left.
  • 2 cases combining both.

Total 685 oracle comparisons + 30 pad-sentinel checks = 715, which is exactly the delta.

Crucially, those 715 checks also pass, with the identical 3067 total, against the pre-refactor code — verified by stashing both repos' source changes and rebuilding the new test file against the old ContiguousStrides/VOIDPTR implementation. They pin existing behaviour; they do not encode the new implementation.


Bonus: the entry points got materially smaller

Removing ContiguousStrides' conditional heap allocation and its RAII/exception scaffolding shrinks the exported functions (clang -O2, instruction counts):

symbolbasenew
ff::cpu::dt_euclidean(DLTensor&, double, int)328172
ff::cpu::dt_l1(DLTensor&, double, int)318150

A per-call saving, entirely outside the per-line loop. The metadata copy itself is not new — the old code also built a copy_meta carrier, just one layer down inside dt().


Judgment calls

The _dt_l1/_dt_euclidean shims stay, rather than collapsing into the dtype arms. The issue offered either. I kept them because DISPATCH_DT passes the dtype as the leading explicit template argument (func<float>), while the new dt() takes the carrier as its first template parameter — so collapsing the shims would have forced a rewrite of DISPATCH_DT, whose shape and exact rejection message the issue calls behavioural ABI. A shim whose first parameter is scalar_t leaves the macro untouched at zero cost. They are no longer void* re-casts: the import is now their entire body, which is a better justification for their existence than they had before.

A new (benign) failure mode worth stating.from_dlpack debug-asserts that the tensor's device_type matches the carrier's memory space. A kDLCUDA tensor handed to ff::cpu::dt_* now trips that assert in a non-NDEBUG build, where previously it would have dereferenced a device pointer on the host. Strictly an improvement, and unreachable in practice (fastfields-lib dispatches on device before reaching ff::cpu), but it is a behaviour change.


Generated by Claude Code

Phase A of the tensor-native-boundaries umbrella (fastfields-lib#57), the
cpu-lib half of one coordinated change with fastfields-cpu-impl#58 (which
converts `dt()` to take the carrier). Convention: TEENY-MIGRATION.md sec. 9.
dt_l1/dt_euclidean held a complete tensor and disassembled it by hand to feed
the impl's raw-args signature. `tny::from_dlpack` does all three parts by
construction -- folds `byte_offset` into the pointer (was VOIDPTR), expands a
NULL `strides` to row-major (was ContiguousStrides), and copies the metadata
into the carrier -- so the carrier is built once, here in the existing dtype
dispatch, and handed straight down.
`ContiguousStrides` stays in autocast.h: the spline/mesh paths in this same
file still use it (Phases D/E; last-user rule). The l1/euclidean paths already
had no int32 arm, so there is no narrowing to delete (R5 pre-applied).
Preserved and checked: the exported signatures are byte-for-byte unchanged
(`_ZN2ff3cpu5dt_l1ER8DLTensordi`, `_ZN2ff3cpu12dt_euclideanER8DLTensordi`);
DISPATCH_DT's shape and its exact rejection message are untouched, and the
evaluation order (lanes check -> dtype switch throw) still reaches the throw
before any import; `<teeny/dlpack.h>` is included AFTER this repo's vendored
"dlpack.h" so the TU stays on DLPack v1.2 throughout (R7).
test_distance gains test_descriptor_variants: NULL strides and byte_offset !=
0, each against the brute-force oracle, plus a sentinel proving nothing before
byte_offset is written. `strides == NULL` is listed in MIGRATION.md as a
historical soft spot and had no test at all. The suite goes 2352 -> 3067
checks; the added 715 pass identically on the pre-refactor code, so they pin
behaviour rather than encode the new implementation.
Closes#72
impl: e401bf2 -> 21833c4, now that fastfields-cpu-impl#60 (dt() taking
a teeny carrier directly) has landed on teeny.
@balbasty
balbasty merged commit 0b03229 into teenyAug 4, 2026
1 check passed
@balbasty
balbasty deleted the claude/72-distance-from-dlpack branch August 4, 2026 05:13
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