Fix: initialize DSv4 decode buffers on device and bound hc_head_linear to valid rows - #1922
Merged
ChaoZheng109 merged 4 commits intoAug 20, 2026
Conversation
…ator Replace the six orchestration-side `set_initial_value(0)` calls in the DeepSeek-V4 FLASH decode cases with device-side initialization, so the TMR and HBG orchestrations are identical here and the host never writes a GM-heap device address. Under HBG that write segfaults the chip subprocess, which is why the calls were simply dropped when the case landed on main — leaving the `mixes_raw` AtomicAdd destination undefined. - `h_tile_i8` padding rows: each of the five `sh_gate_up_act_q*` AIV kernels clears the two padded rows owned by its logical block (`zero_h_tile_i8_padding`) before producing its output. - `mixes_raw` split-K destination: a new `hc_head_mixes_zero` AIV kernel (func_id 367) clears one up-to-16-row chunk per logical block; an explicit task dependency orders every `hc_head_linear` AtomicAdd store after the seed task. `decode_fwd.cpp`, `decode_fwd_hostbuild.cpp` and `decode_fwd_graph.cpp` submit it in place of the plain `alloc_tensors` + `set_initial_value`. Both full-network cases remain `manual` (368-kernel compile); READMEs and docstrings are updated for the new kernel count and for the closed HBG initialization gap. Ported from hw-native-sys#1899, which landed on `perf/hbg-orch` rather than `main`.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughDeepSeek-V4 initialization moved from host tensor setup to device-side kernels. Split-K accumulation now waits for an explicit zeroing task. Gate-up kernels clear padded INT8 rows. Documentation and kernel metadata now report 368 kernels. ChangesDeepSeek-V4 initialization flow
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR moves decode-buffer initialization onto the device to avoid undefined AtomicAdd destinations while preserving the existing orchestration flow. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant HostGraph
participant AIVTask367
participant mixes_raw
participant hc_head_linear
HostGraph->>AIVTask367: Submit zero-initialization task
AIVTask367->>mixes_raw: Write zero values
HostGraph->>hc_head_linear: Register task 367 dependency
AIVTask367-->>hc_head_linear: Signal completion
hc_head_linear->>mixes_raw: Perform split-K atomic additions
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The HBG DSv4 test points at `decode_fwd_graph.cpp`, the Graph form. Its docstring and ring-sizing comment still described the predicated-dispatch variant with a static tile grid, which the README's ruled-out table records as an earlier form that stalls at the same task and is not what the case builds. The ring sizing itself is unchanged: `HBG_RECV_ROWS_PER_EXPERT` holds every per-expert tile loop at a constant trip count, so tile scratch is still allocated for all 32 experts per MoE layer.
ChaoZheng109
changed the title
`x_flat [8,16384]` is a valid 512 KiB allocation, but both TLOAD views were hard-coded as `Shape<...,16,256>` with a 16384-element row stride, covering an almost 1 MiB range and reading rows 8-15 out of bounds. HBG's exact-size per-tensor allocation surfaces the over-read as an MTE out-of-range fault at task 15959; TMR's retained bump allocation keeps it inside a larger mapping and masks the same kernel bug. Derive `valid_rows = clamp(t_dim - row_base, 0, 16)` and use it for the two `x_flat` views and load tiles, the dependent matmul and accumulator tiles, and the `mixes_raw` AtomicAdd store. Cube tile capacity stays at 16. The HBG README records the completed non-Graph device body, corrects the earlier 13-of-16 dispatch reading of the stall, and keeps the original failure signature under its own heading. Ported from hw-native-sys#1871, which landed on `perf/hbg-orch` rather than `main`. The teardown-budget reference is respelled as `_CLOSE_CHILD_REAP_TIMEOUT_S`, whose branch-local commit id does not exist on `main`.
`decode_fwd_hostbuild.cpp` was never a build input: the case points at `decode_fwd_graph.cpp`, and nothing else read the file. Its investigation value is spent now that the tail fault it was used to isolate is root-caused and fixed in `hc_head_linear`. The README loses the stall narrative that only described this form — the per-task dump, the ruled-out table and the two-orchestration provenance — and keeps the Graph-activation blocker, the reason the kernel carries a row-tail bound, and the `get_tensor_data` runtime gap.
ChaoWao
pushed a commit
that referenced
this pull request
Aug 20, 2026
set_initial_value segfaulted the orchestrator on every host_build_graph path. fill_tensor_initial_value memcpy'd to ChipTensor::buffer.addr, and on this runtime that is a GM-heap device address the orchestrating host cannot load. The file is a verbatim copy of the tensormap_and_ringbuffer one, where the orchestrator runs on the AICPU and the store is correct; the copy kept the store and lost its premise. Removing it from this runtime rather than making it work, because there is nothing left to serve. #1922 moved the DeepSeek-V4 case — the only caller that needed it here — to device-side initialization, and the sole remaining caller in the tree is the tensormap_and_ringbuffer scalar_data example, which demonstrates the API itself. Supporting it would also stop short of the case it was for: an initial value cannot survive Graph Execution, where each submission materializes its outputs at addresses it derives for itself from a heap block whose prior contents it never reads, so a value written once while recording reaches none of the replays. tensormap_and_ringbuffer keeps the fill. Its orchestrator runs on the AICPU, which addresses the GM heap directly, so the feature works there and its example and golden are untouched. The two runtimes hold separate copies of this header, so the removal is theirs alone. Dropping the method rather than failing at run time means an orchestration that asks for it does not compile against host_build_graph, instead of building and faulting on device. The two fields occupied bytes [0, 16), aligned with ChipTensor::buffer so init_tensor_from_create_info can cover cache line 1 in one 64-byte memcpy, and start_offset has to stay at byte 24 for the offsetof assertions that follow. Explicit padding holds the layout, and a new assertion pins its intent — offsetof(TensorCreateInfo, __pad0__) == offsetof(ChipTensor, buffer) — so the span is verified rather than described. Those bytes were already dead: the memcpy is followed immediately by t.buffer = {addr, buffer_size}. Verified on a2a3 hardware, including the tensormap_and_ringbuffer scalar_data example, which is the one place in the tree that still sets an initial value and the only regression surface for the shared header this change touches. Also a2a3sim, a5sim, and the C++ unit tests.
5 tasks
ChaoZheng109
added a commit
that referenced
this pull request
Aug 24, 2026
…_and_ringbuffer (#1975) TensorCreateInfo::set_initial_value filled a fresh output inside the hidden alloc task, where nothing could race the write. host_build_graph dropped the same fill in #1930 because its host orchestrator cannot store to the GM-heap device address; the two runtimes then held different API surfaces for the same create-info header. This removes the fill here too, so an orchestration written against one runtime compiles against the other. The only caller in the tree is the scalar_data example, which seeds a 1-element scalar. It now allocates first and writes with set_tensor_data: the alloc task completes inline in the orchestrator before any producer or consumer exists, so the write finds nothing to wait for and runs immediately. Goldens are unchanged. Whole-buffer initialization such as zeroing a large accumulator is not served by per-element set_tensor_data; a task writes the buffer, with task dependencies ordering every reader and writer after the seed — the pattern #1922 established for the DeepSeek-V4 decode buffers. The two occupied bytes [0, 16) are replaced by explicit __pad0__ padding aligned with ChipTensor::buffer, and a new offsetof assertion pins that span, mirroring the host_build_graph layout. The stale cross-reference in the shared tensor.h comment goes with it. Verified: scalar_data on a2a3sim and a5sim with golden comparison (both PASSED), the full C++ unit-test suite (117/117), and the Python unit tests (1892 passed, 20 skipped).
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
1 participant
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.
What
Ports #1899 and #1871 to
main. Both landed onperf/hbg-orch, somainstillcarries the workaround they replaced: the DeepSeek-V4 FLASH decode cases dropped
their six orchestration-side
set_initial_value(0)calls to avoid a segfaultunder HBG, which left the
mixes_rawAtomicAdd destination undefined and gaveup numerical correctness — and
hc_head_linearstill reads past the end ofx_flat, which is what stalls the non-Graph device body 12 tasks from the end.Device-side initialization (#1899)
The six initializations move from the orchestrator to the device, so the TMR and
HBG orchestrations are identical here and the host never writes a GM-heap device
address.
h_tile_i8padding rows: each of the fivesh_gate_up_act_q*AIV kernelsnow clears the two padded rows owned by its logical block
(
zero_h_tile_i8_padding) before producing its output.mixes_rawsplit-K destination: a newhc_head_mixes_zeroAIV kernel(func_id 367) clears one up-to-16-row chunk per logical block; an explicit
task dependency orders every
hc_head_linearAtomicAddstore after the seedtask. Both orchestrations (
decode_fwd.cpp,decode_fwd_graph.cpp) submit itin place of the plain
alloc_tensors+set_initial_value.This closes the "AtomicAdd destinations cannot currently be initialized
correctly by HBG" runtime gap recorded in the HBG README.
hc_head_linearrow-tail bound (#1871)x_flat [8,16384]is a valid 512 KiB allocation, but both TLOAD views werehard-coded as
Shape<...,16,256>with a 16384-element row stride — an almost1 MiB range, reading rows 8–15 out of bounds. HBG's exact-size per-tensor
allocation surfaces that as an MTE out-of-range fault at task 15959; TMR's
retained bump allocation keeps it inside a larger mapping and masks the same
kernel bug.
valid_rows = clamp(t_dim - row_base, 0, 16)now bounds the twox_flatviewsand load tiles, the dependent matmul and accumulator tiles, and the
mixes_rawAtomicAdd store. Cube tile capacity stays at 16. This is the root cause of the
tail stall the HBG README documented, so that status section is rewritten around
what remains open: Graph activation.
Both full-network cases remain
manual(368-kernel compile); READMEs anddocstrings are updated for the new kernel count and status.
Differences from the branch versions
Two doc claims on
perf/hbg-orchdo not hold onmainand were rewrittenrather than copied:
342e29f1, which is branch-localand unreachable from
main. The fix itself is onmain, so the note nownames
_CLOSE_CHILD_REAP_TIMEOUT_Sand its 60 s value instead of a sha.0cefc9a5pin" phrasing keep
main's own wording ("blocked on the CI: bump pinned pto-isa for PTOAS v0.55 #1644 pto-isa pinbump" / "including current main") —
mainpinscd4a3d3f.Relatedly, the stale
(= pto_isa.pin)claim next to the generation ISA isdropped, as in #1899:
maindoes not pin the83d01313these kernels weregenerated against.
Also in this PR
decode_fwd_hostbuild.cppis deleted. It was never a build input — the casepoints at
decode_fwd_graph.cppand nothing else read the file — and itsinvestigation value is spent now that the tail fault it isolated is root-caused
and fixed above. The HBG README loses the stall narrative that described only
that form (the per-task dump, the ruled-out table, the two-orchestration
provenance) and keeps the Graph-activation blocker, the reason the kernel
carries a row-tail bound, and the
get_tensor_dataruntime gap.One commit also fixes two stale comments in the HBG test that predate this change
(they came in with #1900): the docstring and the ring-sizing comment still named
the predicated-dispatch variant with a static tile grid, while the case actually
builds
decode_fwd_graph.cpp— the predicated form was an earlier, discardedvariant. No behavior or sizing change.
Test
Both full-network cases are
manualand, onmain, still blocked mid-networkby the pinned pto-isa (documented in the TMR README), so an end-to-end run
proves nothing here. The device evidence for the row-tail fix is in #1871
(
task_20260817_204500_135435017977,task_20260817_210320_91023923204— bothranks
outcome=0,PASSED, no MTE fault, 507018,TIMEOUT_EXITor task-15959stall).
Compile coverage against main's pinned ISA (
cd4a3d3f) was verified here:hc_head_mixes_zero.cpp, all five modifiedsh_gate_up_act_q*.cppand the modified
hc_head_linear.cppcompile fora2a3(ccec, AIV/AIC);decode_fwd.cppand HBGdecode_fwd_graph.cpp—compile for their runtimes.
Full
pre-commitrun over the changed files passes.