es-ude / es-ude/OnDeviceTraining

executeOp: zero the Phase-2 raw for accumulate-style kernels (scatter correctness is stack-layout-dependent)

Open
#427 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
1
Forks
3
Avg merge
1d 1h
Merged PRs (30d)
8

Description

Design decision surfaced by the BFP epic PR4 wave-2 review (FU-1): the correctness of every scatter-style backward kernel currently depends on the kernel remembering to `memset` the funnel's Phase-2 raw — and a FORGOTTEN memset is not reliably testable.

**Mechanism:** `executeOp` sizes the Phase-2 raw as a stack VLA (`ExecuteOp.c:486` on the PR4 branch) and hands it to the kernel uninitialized. Scatter kernels (the three PR4 BFP dx kernels, the SYM backward kernels in all three pool layers, Conv1d's weight-grad kernel) accumulate into it and must zero it first. Whether a dropped memset is observable depends on what the prologue's callee frames left at that stack depth — PR4's instrumented builds saw both all-zeros AND garbage (−1.0e31) on a FIRST call, so a green test proves nothing about the memset. PR4's dx value tests mitigate by calling the backward twice (pinning OUT_WRITE idempotence — a real contract, but a probabilistic kill).

**Options:**
- `executeOp` zeroes the raw for accumulate-style kernels (simplest; costs a memset per op for kernels that overwrite anyway), or
- an `opSpec_t` flag (`zeroRaw`/`accumulatesIntoRaw`) so only scatter kernels pay, with the flag itself testable.

Repo-wide question (touches the funnel contract in `docs/conventions/`), so it needs a deliberate decision rather than a drive-by fix. Evidence and per-layer kill-status notes: PR4 wave-2 report + the dx-test comments in `test/unit/layer/UnitTest{AvgPool1d,AdaptiveAvgPool1d,MaxPool1d}.c`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01UzyrYp4e8J2tRjAbkhtnwo

Contributor guide

Open the contributing guide

Research direction

Start with ExecuteOp.c:486 and the funnel contract in docs/conventions/, then read the PR4 wave-2 report and the dx-test comments in test/unit/layer/UnitTest{AvgPool1d,AdaptiveAvgPool1d,MaxPool1d}.c. Compare the listed scatter kernels and evaluate the two zeroing options. Done means the contract decision is recorded, the chosen behavior is implemented consistently, and tests can reliably detect a missing zeroing step.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
documentation, embedded-iot, testing-qa
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.