Skip to content

feat(cuda): PR103.0 GDN cache-cpy fusion — A/B toggle + test coverage (implements #112) - #115

Open
ddvnguyen wants to merge 7 commits into
baselinefrom
fork/pr103-gdn-fusion-impl
Open

ddvnguyen wants to merge 7 commits into
baselinefrom
fork/pr103-gdn-fusion-impl

Conversation

@ddvnguyen

Copy link
Copy Markdown
Owner

Implements the codeable delta of the PR #112 arm spec on a fresh branch off fork/pr103-gdn-fusion-arm.

Important correction to the #112 premise

The GDN -> cache-cpy fusion already exists on CUDA in this baseline: upstream ggml-org#23940 (5a460de, "Remove redundant CUDA copies after gated_delta_net") added ggml_cuda_try_gdn_cache_fusion (ggml-cuda.cu) + the fused kernel path, months before the Metal-side GGML_METAL_FUSE_GDN_CACHE commit (d011a21) that #112 was modeled on. #112's claim that "CUDA still runs the unfused GDN + cpy" was wrong — the Metal commit is not even in this baseline's history. What was actually missing, and what this PR adds:

  1. GGML_CUDA_FUSE_GDN_CACHE toggle (ggml-cuda.cu, in ggml_cuda_try_gdn_cache_fusion): =0 disables only this fusion, default on. The pre-existing GGML_CUDA_DISABLE_FUSION kills every CUDA fusion, which would contaminate the PR103.0 A/B (moe weighted reduction, topk-moe, etc. would also turn off). One binary now serves fused and unfused arms.
  2. test_gated_delta_net_cache_fusion ported into tests/test-backend-ops.cpp (5 cases) — this test landed upstream only with the Metal commit, outside this baseline. Whole-graph CUDA run takes the fused path; the elided cpy's dst (cache view) is compared against the CPU reference.

Kernel and fusion matcher are untouched — no new kernels, no graph changes, no ggml-core changes, per #112 non-goals.

Validation (RTX 3060 sm_86, CUDA 13, local rig — 5060 Ti was fully occupied)

  • Fusion ON (default): 5/5 GATED_DELTA_NET_CACHE_FUSION cases OK; debug log confirms the fused path fires per case (fused gated_delta_net snapshot copies ... skipped 3 nodes)
  • GGML_CUDA_FUSE_GDN_CACHE=0: 5/5 OK via the unfused path, zero fusion hits — toggle verified
  • Regression: 38/38 existing GATED_DELTA_NET cases OK
  • Byte-identical gate (standalone harness, same graph/inputs, fusion toggled across two runs, raw buffers dumped and cmp'd): fused cache buffer == unfused cache buffer == unfused gdn tail, bit-for-bit; attention output identical across runs. The fused run's gdn-output tail region is intentionally unwritten (kernel writes state straight to the cache; the matcher guarantees the only tail consumer — the elided cpy — and that the gdn output is not a graph output), matching the Metal-side design.

Remaining for the arm (rig, later)

  • Greedy run-to-run determinism cold/warm + fused-vs-unfused greedy parity on the real model (the toggle enables this directly)
  • Perf A/B vs bars — note PR105.0's single-machine topology underperformed the RPC production topology, so the reference topology for the A/B needs a decision before measuring (rig-execution concern, not a code concern)

Test plan

AI usage disclosure: YES — implemented by AI (opencode) as a handoff task; the human reviewer is expected to verify the toggle placement and test port against ggml-cuda.cu:2759 and the upstream ggml-org#23940/#d011a214b commits. All work stays on the ddvnguyen fork; nothing pushed to upstream origin.

Hydra Engineering added 7 commits September 10, 2026 00:39
…tests

The gated_delta_net -> cpy fusion itself already exists on CUDA (upstream ggml-org#23940,
5a460de); what was missing vs the PR #112 arm spec is a fusion-specific toggle
and the test coverage that landed upstream only with the later Metal-side fusion
commit (d011a21), which is not part of this baseline.

- GGML_CUDA_FUSE_GDN_CACHE=0 disables only the gdn cache-cpy fusion (default on),
  so one binary A/Bs fused vs unfused without GGML_CUDA_DISABLE_FUSION killing
  every other CUDA fusion.
- Port test_gated_delta_net_cache_fusion into test-backend-ops (5 cases): the
  whole-graph CUDA run takes the fused path and the cache view (the elided cpy's
  dst) is compared against the CPU reference.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant