feat(arithmetic)!: executeOp scratch sized per converted operand + OUT_WRITE aliasing (#296 Stage 1)#334
Merged
Merged
Conversation
a33b8f3 to
38b2797
Compare
Independent review (fable) of #325 found two real bugs in the gate job: - run: steps use bash -e by default; an unhandled gh pr list failure (API hiccup) would abort the script before `proceed` is ever written, failing the gate job itself -- the exact red-X-on-transient-flake outcome #325 was meant to eliminate, now just relocated to api.github.com instead of apt.llvm.org. `|| dup=0` makes a lookup failure fail open (full run proceeds) instead of failing the gate. - `gh pr list` defaults to --limit 30; past that many concurrently open PRs, a real duplicate could go undetected (fails open the same way, just silently -- harmless but worth closing). --limit 100 covers this repo's realistic ceiling.
…ing + OUT_WRITE aliasing (#296 Stage 1) - executeOp prologue: one summed scratch VLA sized to the operands that actually convert (zero bytes when all dtypes match the arithmetic), replacing the unconditional scratch[3][maxElems*4] - rawData aliasing: FLOAT32-arithmetic -> FLOAT32-target OUT_WRITE kernels write the target directly; gated on no post-prologue operand aliasing the target unless the new opSpec_t.writesInPlaceSafe (elementwise contract) is set; never for OUT_ACC (staging load-bearing) or non-FLOAT32 targets (width-restore load-bearing) - the three SGD update kernels opt in (verified elementwise) - stack-watermark budgets recalibrated to measured post-change peaks: float 99560 -> 27328 B (peak moved to conv3 weight-grad OUT_ACC), sym 149008 -> 100080 B (repack chain remains for Stage 2); pytest fixtures updated to the new measured values Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
38b2797 to
a059828
Compare
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 free
to 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.
Stage 1 of the bounded-scratch design for #296 (spec:
docs/superpowers/specs/2026-07-09-bounded-scratch-design.md; Stage 2 — O(chunk) conversions — follows as its own PR). Executed via SDD: per-task reviews + final whole-branch review (verdict: merge-ready).What changes
executeOpprologue scratch is sized per actually-converted operand — one summed VLA, each row sized to its own operand, zero bytes when every operand already matches the arithmetic dtype (the entire FLOAT32 training path). Previously:scratch[3][maxElems·4]unconditionally — 73,728 B at HAR's conv3 even when nothing converted.rawDataoutput-aliasing: FLOAT32-arithmetic → FLOAT32-targetOUT_WRITEops let the kernel write the target directly (the epilogue there was a same-type memmove). Gated hard: never forOUT_ACC_*(accumulate must keep the intermediate distinct), never for non-FLOAT32 targets (the SYM→SYM epilogue is a load-bearing width-restoring requant), and never when the target's data pointer equals a post-prologue operand pointer — unless the newopSpec_t.writesInPlaceSafe(zero-init false) declares the kernel elementwise. The three SGD update kernels opt in (verified strictly elementwise).Measured effect (HAR, EPOCHS=1, macOS arm64)
OUT_ACC(rawData 24,576 + ~2.7 K frames) — staging there is load-bearing, aliasing an accumulate is semantically impossibleProjection after Stage 2: sym ≈ 51 KB → total training RAM sym8 ≈ 205 KB vs float ≈ 212 KB — the #322 inversion flips in sym's favor at HAR scale.
Safety argument for the aliasing (reviewers' conjunct→killer map)
OUT_WRITEconjunct →testAccDynamicIntoFloatIsExactAndAccumulatesgoes RED (aliased ACC doubles the increment).ARITH_FLOAT32conjunct →testOutWriteSymToFloatEqualsConvertTensorgoes RED (raw mantissas reach the float target).testExecuteOpNeverAliasesSymTargetgoes RED (SYM width-restore skipped).testExecuteOpDoesNotAliasWhenTargetIsAnInputgoes RED (mutation-verified during Task 1).Known contract edge (documented, deferred to Stage 2's doc pass): the self-alias check is exact-base-pointer equality — overlapping sub-views (constructible only via raw-pointer
tensorInitFloatwiring) are outside the contract.Verification
ctest --preset unit_test_debug72/72; focusedUnitTestExecuteOp26/26 (4 new aliasing tests + 1 new 3-input mixed-dtype prologue pin);uv run pytest29/29;cmake --build --preset examplesall targets.Refs #296 (Stage 2 PR will close it), #322 (stack narrative), #300 axis-1 addendum.
🤖 Generated with Claude Code