Restore DeepSeek4 HIP backend safely by howard0su · Pull Request #514 · Luce-Org/lucebox

howard0su · 2026-07-12T12:24:08Z

Restore the DeepSeek4 full-HIP backend while keeping the active request path stable and preserving batched prefill throughput.

Key changes:

Re-enable cfg-driven batched prefill chunks for full-HIP and layer-split paths instead of forcing single-token prefill.
Keep the cached single-token attention graph disabled on HIP because the ggml_set_rows state-update path faults on longer prompts.
Snapshot existing raw SWA rows before writing the current batched chunk into the ring, avoiding stale/future KV reads while retaining batched execution.
Reset full-HIP request state between requests, preserve last logits in snapshots, and restore exact-prefix generation without extra prefill.
Propagate BudgetHook force-close behavior and structured GenerateResult errors through fresh and restored generation paths.

Validation:

Local: server/build/test_deepseek4_unit and server/build/test_server_unit passed.
Remote HIP: build-hip/test_deepseek4_unit passed on howardsu@100.126.37.90.
Remote full-HIP perf with --chunk 512: 525 prompt tokens at 105.21 tok/s, 1045 prompt tokens at 109.44 tok/s.

Restore the DeepSeek4 full-HIP backend while keeping the active request path stable and preserving batched prefill throughput. Key changes: - Re-enable cfg-driven batched prefill chunks for full-HIP and layer-split paths instead of forcing single-token prefill. - Keep the cached single-token attention graph disabled on HIP because the ggml_set_rows state-update path faults on longer prompts. - Snapshot existing raw SWA rows before writing the current batched chunk into the ring, avoiding stale/future KV reads while retaining batched execution. - Reset full-HIP request state between requests, preserve last logits in snapshots, and restore exact-prefix generation without extra prefill. - Propagate BudgetHook force-close behavior and structured GenerateResult errors through fresh and restored generation paths. Validation: - Local: server/build/test_deepseek4_unit and server/build/test_server_unit passed. - Remote HIP: build-hip/test_deepseek4_unit passed on howardsu@100.126.37.90. - Remote full-HIP perf with --chunk 512: 525 prompt tokens at 105.21 tok/s, 1045 prompt tokens at 109.44 tok/s.

cubic-dev-ai

9 issues found across 22 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="server/src/deepseek4/deepseek4_backend.cpp">

<violation number="1" location="server/src/deepseek4/deepseek4_backend.cpp:146">
P2: Plain `park`/`unpark` and `park all`/`unpark all` now silently do nothing for this backend because both methods reject selectors the daemon interface defines as valid. Accept empty and `all` as target requests so standard lifecycle commands still release/reload the model.</violation>

<violation number="2" location="server/src/deepseek4/deepseek4_backend.cpp:406">
P2: Restored generation bypasses `req.on_token`, so callback-based streaming and disconnect cancellation do not work on cache hits. Route prefill/decode through `io.with_token_callback(req.on_token)`, matching the other backend paths.</violation>
</file>

<file name="server/src/deepseek4/deepseek4_layer_split_adapter.cpp">

<violation number="1" location="server/src/deepseek4/deepseek4_layer_split_adapter.cpp:483">
P2: Local multi-shard forwards invoke undefined behavior when passing HC output to the next shard: the next step memcpy's `hc_state_` onto itself. Preserve the boundary in a separate buffer or make the layer-range copy conditional when pointers match.</violation>
</file>

<file name="server/src/qwen35/qwen35_layer_split_adapter.h">

<violation number="1" location="server/src/qwen35/qwen35_layer_split_adapter.h:63">
P2: Intermediate prefill chunks always compute and transfer full logits even when need_logits is false. The forward function conditionally skips logit work when its logits_out pointer is null, so passing nullptr for intermediate chunks would save the vocab×tokens matmul and GPU→CPU transfer on every chunk except the last.</violation>
</file>

<file name="server/CMakeLists.txt">

<violation number="1" location="server/CMakeLists.txt:438">
P2: CUDA builds now always take the full-logits D2H/CPU top-K fallback because dispatch still checks `DFLASH27B_HAVE_DRAFT_TOPK`. Keep the existing macro name here or update every consumer guard to the renamed CUDA-specific name.</violation>
</file>

<file name="server/src/laguna/laguna_layer_split_adapter.cpp">

<violation number="1" location="server/src/laguna/laguna_layer_split_adapter.cpp:845">
P2: Chunked prefill still requests and copies logits for every intermediate chunk because `need_logits` is discarded. Honor the flag so only prompt-ending and snapshot-ending chunks pay for the vocabulary projection, preserving intended batched-prefill throughput.</violation>
</file>

<file name="server/src/deepseek4/deepseek4_graph.cpp">

<violation number="1" location="server/src/deepseek4/deepseek4_graph.cpp:987">
P0: Batched prefill skips compressor updates for every token except the chunk's last token. Subsequent decode treats all crossed ratio windows as cached, so attention reads stale/uninitialized `comp_kv` (and `index_comp_kv` for ratio 4); build sequential compressor updates per token/window before advancing counts.</violation>

<violation number="2" location="server/src/deepseek4/deepseek4_graph.cpp:3229">
P2: CUDA decode now rebuilds its attention graph for every token because this disables reuse on every backend, not only HIP. Preserve cached decode attention for non-HIP backends while retaining the HIP workaround.</violation>
</file>

<file name="server/src/gemma4/gemma4_layer_split_adapter.cpp">

<violation number="1" location="server/src/gemma4/gemma4_layer_split_adapter.cpp:1030">
P2: The `need_logits` parameter is passed by the caller (layer_split_backend.cpp:92) to avoid computing logits on intermediate chunks, saving the projection computation on the last token of each non-terminal chunk. The current `(void)need_logits;` ignores it and always captures logits via `&prefill_last_logits_`, which means intermediate chunks unconditionally compute an unused projection.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

cubic-dev-ai · 2026-07-12T12:35:36Z

+    }
+    std::vector<int32_t> remaining(req.prompt.begin() + snap_pos, req.prompt.end());
+    const auto t0 = Clock::now();
+    const int committed = remaining.empty() ? snap_pos : do_prefill(remaining, io, snap_pos);


P2: Restored generation bypasses req.on_token, so callback-based streaming and disconnect cancellation do not work on cache hits. Route prefill/decode through io.with_token_callback(req.on_token), matching the other backend paths.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_backend.cpp, line 406: <comment>Restored generation bypasses `req.on_token`, so callback-based streaming and disconnect cancellation do not work on cache hits. Route prefill/decode through `io.with_token_callback(req.on_token)`, matching the other backend paths.</comment> <file context> @@ -715,56 +362,87 @@ void DeepSeek4Backend::snapshot_free(int slot) { + } + std::vector<int32_t> remaining(req.prompt.begin() + snap_pos, req.prompt.end()); + const auto t0 = Clock::now(); + const int committed = remaining.empty() ? snap_pos : do_prefill(remaining, io, snap_pos); + if (committed < 0) { + result.fail(GenerateErrorCode::PrefillFailed); </file context>

cubic-dev-ai · 2026-07-12T12:35:36Z

-        free_deepseek4_snapshot(snapshots_[i]);
-    }
-    last_logits_.clear();
+    if (what != "target") return false;


P2: Plain park/unpark and park all/unpark all now silently do nothing for this backend because both methods reject selectors the daemon interface defines as valid. Accept empty and all as target requests so standard lifecycle commands still release/reload the model.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_backend.cpp, line 146: <comment>Plain `park`/`unpark` and `park all`/`unpark all` now silently do nothing for this backend because both methods reject selectors the daemon interface defines as valid. Accept empty and `all` as target requests so standard lifecycle commands still release/reload the model.</comment> <file context> @@ -309,233 +104,68 @@ DeepSeek4Backend::~DeepSeek4Backend() { - free_deepseek4_snapshot(snapshots_[i]); - } - last_logits_.clear(); + if (what != "target") return false; free_deepseek4_cache(cache_); - stream_engine_.destroy(); </file context>

cubic-dev-ai · 2026-07-12T12:35:36Z

            return false;
        }
        if (timing) add_step_tel(tel_acc, step_tel);
+        shard_input = hc_state_.data();


P2: Local multi-shard forwards invoke undefined behavior when passing HC output to the next shard: the next step memcpy's hc_state_ onto itself. Preserve the boundary in a separate buffer or make the layer-range copy conditional when pointers match.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_layer_split_adapter.cpp, line 483: <comment>Local multi-shard forwards invoke undefined behavior when passing HC output to the next shard: the next step memcpy's `hc_state_` onto itself. Preserve the boundary in a separate buffer or make the layer-range copy conditional when pointers match.</comment> <file context> @@ -479,13 +473,14 @@ bool DeepSeek4LayerSplitAdapter::run_forward( return false; } if (timing) add_step_tel(tel_acc, step_tel); + shard_input = hc_state_.data(); } </file context>

cubic-dev-ai · 2026-07-12T12:35:36Z

    bool prefill(const std::vector<int32_t> & prompt,
-                 int base_pos, int & last_tok) override;
+                 int base_pos, int & last_tok,
+                 bool need_logits = true) override;


P2: Intermediate prefill chunks always compute and transfer full logits even when need_logits is false. The forward function conditionally skips logit work when its logits_out pointer is null, so passing nullptr for intermediate chunks would save the vocab×tokens matmul and GPU→CPU transfer on every chunk except the last.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/src/qwen35/qwen35_layer_split_adapter.h, line 63: <comment>Intermediate prefill chunks always compute and transfer full logits even when need_logits is false. The forward function conditionally skips logit work when its logits_out pointer is null, so passing nullptr for intermediate chunks would save the vocab×tokens matmul and GPU→CPU transfer on every chunk except the last.</comment> <file context> @@ -59,7 +59,8 @@ class Qwen35LayerSplitAdapter : public LayerSplitAdapter { bool prefill(const std::vector<int32_t> & prompt, - int base_pos, int & last_tok) override; + int base_pos, int & last_tok, + bool need_logits = true) override; bool decode_ar(int last_tok, int committed, int n_gen, std::vector<int32_t> & out_tokens, </file context>

cubic-dev-ai · 2026-07-12T12:35:36Z

-    # name as the HIP branch above (backend-neutral).
-    target_compile_definitions(dflash_common PUBLIC DFLASH27B_HAVE_DRAFT_TOPK=1)
+    # and take the GPU draft top-K path instead of the CPU fallback.
+    target_compile_definitions(dflash_common PUBLIC DFLASH27B_HAVE_DRAFT_TOPK_CUDA=1)


P2: CUDA builds now always take the full-logits D2H/CPU top-K fallback because dispatch still checks DFLASH27B_HAVE_DRAFT_TOPK. Keep the existing macro name here or update every consumer guard to the renamed CUDA-specific name.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/CMakeLists.txt, line 438: <comment>CUDA builds now always take the full-logits D2H/CPU top-K fallback because dispatch still checks `DFLASH27B_HAVE_DRAFT_TOPK`. Keep the existing macro name here or update every consumer guard to the renamed CUDA-specific name.</comment> <file context> @@ -447,9 +434,8 @@ elseif(DFLASH27B_GPU_BACKEND STREQUAL "cuda") - # name as the HIP branch above (backend-neutral). - target_compile_definitions(dflash_common PUBLIC DFLASH27B_HAVE_DRAFT_TOPK=1) + # and take the GPU draft top-K path instead of the CPU fallback. + target_compile_definitions(dflash_common PUBLIC DFLASH27B_HAVE_DRAFT_TOPK_CUDA=1) # GPU port of the sample_logits chain. Compiled in by default; the path is # then opted into at runtime via the DFLASH_GPU_SAMPLE env var. Turn the </file context>

Suggested change

target_compile_definitions(dflash_common PUBLIC DFLASH27B_HAVE_DRAFT_TOPK_CUDA=1)

target_compile_definitions(dflash_common PUBLIC DFLASH27B_HAVE_DRAFT_TOPK=1)

cubic-dev-ai · 2026-07-12T12:35:36Z

+    (void)need_logits;
    const bool ok = use_mixed_target_split()
        ? run_mixed_forward(prompt, base_pos, last_tok, &prefill_last_logits_)
        : run_forward(prompt, base_pos, last_tok, &prefill_last_logits_);


P2: Chunked prefill still requests and copies logits for every intermediate chunk because need_logits is discarded. Honor the flag so only prompt-ending and snapshot-ending chunks pay for the vocabulary projection, preserving intended batched-prefill throughput.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/src/laguna/laguna_layer_split_adapter.cpp, line 845: <comment>Chunked prefill still requests and copies logits for every intermediate chunk because `need_logits` is discarded. Honor the flag so only prompt-ending and snapshot-ending chunks pay for the vocabulary projection, preserving intended batched-prefill throughput.</comment> <file context> @@ -840,7 +840,9 @@ bool LagunaLayerSplitAdapter::run_mixed_forward( - int & last_tok) { + int & last_tok, + bool need_logits) { + (void)need_logits; const bool ok = use_mixed_target_split() ? run_mixed_forward(prompt, base_pos, last_tok, &prefill_last_logits_) </file context>

Suggested change

(void)need_logits;

const bool ok = use_mixed_target_split()

? run_mixed_forward(prompt, base_pos, last_tok, &prefill_last_logits_)

: run_forward(prompt, base_pos, last_tok, &prefill_last_logits_);

const bool ok = use_mixed_target_split()

? run_mixed_forward(prompt, base_pos, last_tok,

need_logits ? &prefill_last_logits_ : nullptr)

: run_forward(prompt, base_pos, last_tok,

need_logits ? &prefill_last_logits_ : nullptr);

cubic-dev-ai · 2026-07-12T12:35:36Z

+            // with ggml_set_rows. On HIP this path can fault after enough prompt
+            // tokens; use the dynamic graph until the cached state-update path is
+            // made safe.
+            const bool reuse_decode_attn = false;


P2: CUDA decode now rebuilds its attention graph for every token because this disables reuse on every backend, not only HIP. Preserve cached decode attention for non-HIP backends while retaining the HIP workaround.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_graph.cpp, line 3229: <comment>CUDA decode now rebuilds its attention graph for every token because this disables reuse on every backend, not only HIP. Preserve cached decode attention for non-HIP backends while retaining the HIP workaround.</comment> <file context> @@ -3023,7 +3222,11 @@ bool deepseek4_step_layer_range( + // with ggml_set_rows. On HIP this path can fault after enough prompt + // tokens; use the dynamic graph until the cached state-update path is + // made safe. + const bool reuse_decode_attn = false; ggml_tensor * attn_out = nullptr; ggml_cgraph * gf = nullptr; </file context>

Suggested change

const bool reuse_decode_attn = false;

const bool reuse_decode_attn = n_tokens == 1 && !ds4_backend_is_hip(backend);

cubic-dev-ai · 2026-07-12T12:35:36Z

+    (void)need_logits;
    const bool ok = use_mixed_target_split()
        ? run_mixed_forward(prompt, base_pos, last_tok, &prefill_last_logits_)
        : run_forward(prompt, base_pos, last_tok, &prefill_last_logits_);


P2: The need_logits parameter is passed by the caller (layer_split_backend.cpp:92) to avoid computing logits on intermediate chunks, saving the projection computation on the last token of each non-terminal chunk. The current (void)need_logits; ignores it and always captures logits via &prefill_last_logits_, which means intermediate chunks unconditionally compute an unused projection.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/src/gemma4/gemma4_layer_split_adapter.cpp, line 1030: <comment>The `need_logits` parameter is passed by the caller (layer_split_backend.cpp:92) to avoid computing logits on intermediate chunks, saving the projection computation on the last token of each non-terminal chunk. The current `(void)need_logits;` ignores it and always captures logits via `&prefill_last_logits_`, which means intermediate chunks unconditionally compute an unused projection.</comment> <file context> @@ -1025,7 +1025,9 @@ bool Gemma4LayerSplitAdapter::run_mixed_forward( - int & last_tok) { + int & last_tok, + bool need_logits) { + (void)need_logits; const bool ok = use_mixed_target_split() ? run_mixed_forward(prompt, base_pos, last_tok, &prefill_last_logits_) </file context>

Suggested change

(void)need_logits;

const bool ok = use_mixed_target_split()

? run_mixed_forward(prompt, base_pos, last_tok, &prefill_last_logits_)

: run_forward(prompt, base_pos, last_tok, &prefill_last_logits_);

const bool ok = use_mixed_target_split()

? run_mixed_forward(prompt, base_pos, last_tok, need_logits ? &prefill_last_logits_ : nullptr)

: run_forward(prompt, base_pos, last_tok, need_logits ? &prefill_last_logits_ : nullptr);

CMake now defines DFLASH27B_HAVE_DRAFT_TOPK_CUDA (CUDA backend only) and no longer defines DFLASH27B_HAVE_DRAFT_TOPK on any backend. Update the consumers in qwen35_dflash_target.cpp and test_dflash.cpp to test the new name so the GPU draft top-K path is actually compiled in on CUDA instead of silently falling back to the CPU heap extract. Fix the stale header comment to match the CUDA-only build.

…pter The layer-split adapter's prefill() hardcoded chunk_size = 1, forcing single-token prefill and defeating the batched layer-range path. This contradicts both the intended behavior (prefill_chunk_tokens() reports cfg_.chunk) and the restore commit's stated goal of cfg-driven batched prefill. Restore chunk_size to cfg_.chunk (default 512).

The learned KV compressor is a sequential accumulator: every token feeds its ratio-window state and each window boundary flushes one pooled comp_kv row (index_comp_kv too for ratio 4). Batched prefill only ran the compressor for the chunk's last token via cur_last, yet advanced n_comp to next_pos/ratio, so every intermediate window's compressed row was left stale/uninitialized while subsequent decode attended over it. Step the compressor for every token in the chunk (matching the single-token decode path); the per-token side effects serialize by graph insertion order, like the raw SWA ring write loop. Scale the dynamic attention graph node budget and metadata arena with n_tokens to hold the per-token compressor chain.

cubic-dev-ai Bot reviewed Jul 12, 2026

View reviewed changes

howard0su added 3 commits July 13, 2026 08:52

howard0su marked this pull request as draft July 13, 2026 01:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Restore DeepSeek4 HIP backend safely#514

Restore DeepSeek4 HIP backend safely#514
howard0su wants to merge 4 commits into
Luce-Org:mainfrom
howard0su:ds4_prefill_batch

howard0su commented Jul 12, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	target_compile_definitions(dflash_common PUBLIC DFLASH27B_HAVE_DRAFT_TOPK_CUDA=1)
	target_compile_definitions(dflash_common PUBLIC DFLASH27B_HAVE_DRAFT_TOPK=1)

	const bool reuse_decode_attn = false;
	const bool reuse_decode_attn = n_tokens == 1 && !ds4_backend_is_hip(backend);

Uh oh!

Conversation

howard0su commented Jul 12, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

howard0su commented Jul 12, 2026 •

edited by cubic-dev-ai Bot

Loading

cubic-dev-ai Bot left a comment •

edited

Loading