perf(deepseek4): enable causal Flash prefill#512
Open
weicj wants to merge 10 commits into
Open
Conversation
Ports the ROCmFP4/ROCmFPX ggml work (previously lucebox-ggml Luce-Org#36, on the old submodule pointer) into the vendored server/deps/llama.cpp tree. - ROCmFP4/ROCmFPX quant types + CPU reference conversions (ggml/rocmfp4, ggml/rocmfpx) and the ggml type-trait registrations. - CUDA/HIP dequant, copy, getrows, MMVQ vecdot, MMVF, unary and FA paths for the new types. - Fused DS4 hyper-connection op (GGML_OP_DS4_HC) with the register-resident sinkhorn kernel; inert unless emitted by the DS4 fused-decode path. - DS4 SwiGLU split op plumbing for the fused FFN matvec paths. Layered on current main, so the main-side ggml work is preserved (fp64 RoPE reduction, Luce-Org#497 RDNA MMQ tile, LUCE_MMQ_DP_MAX_NE1, MMVQ_MAX_MOE_BATCH_SIZE, fused dual set_rows, raw-span guard). Review fixes on top of Luce-Org#36: - ggml_ftype_to_ggml_type: handle the 11 new ROCmFPX ftypes (dominant-type mapping) so the enum switch is -Wswitch/-Werror clean. - FP6 MMVQ vecdot: pad qs[] to avoid a stack over-read of the last window (bit-identical; the over-read bits were already masked out).
The non-HIP fallback of rocmfp4_get_int_from_codebook_16 / rocmfp4_get_low_int_from_codebook_16 called get_int_from_table_16, which is defined in vecdotq.cuh. TUs that pull in this header without vecdotq.cuh (fattn-chunked.cu reaches it via the fattn dequant chain) failed to compile under nvcc: rocmfp4_hip_codebook.cuh: error: identifier "get_int_from_table_16" is undefined The HIP path never hit this (it uses __builtin_amdgcn_perm), so the ROCm CI and the Strix build stayed green while the sm_86 CUDA build broke. Fix: inline the generic table expander (the generic branch of get_int_from_table_16, verbatim) as a static helper in this header, so the fallback no longer depends on include order. Bit-identical; the HIP hot path is unchanged.
Server-side DeepSeek V4 Flash changes from Luce-Org#494, rebased onto the vendored ROCmFPX ggml tree (no submodule pointer). Byte-identical to the validated Luce-Org#494 server files. Correctness (default ON): - Token-by-token prefill by default; chunked prefill only fits inside the raw SWA ring, so long prompts / multi-turn degraded. Chunked stays available via DFLASH_DS4_CHUNKED_PREFILL for short-prompt benchmarking. - Clear the cache buffer at new-sequence prefill (kv_offset==0) so requests 2..N are byte-stable instead of pooling over leftover compressor state. - Route non-hybrid (all-hot) placement through the HC-complete layer-range path; deepseek4_step's non-hybrid branch is HC-less and generates garbage. - Key the cached decode-attn graph on the flush pattern; the old key collided once the compressed-KV ring filled, reusing a stale-topology graph. - Compressor decode graphs read state/comp-cache through the ggml_set_rows result tensors so the current-step writes are explicit graph dependencies. - Default DeepSeek4 chat prefix when the request has no system message, so the ROCmFPX "Src" GGUF stops behaving like a base model under bare prompts (explicit caller system prompts are preserved). Perf, byte-identical default: - Persistent HC matvec pool (row-split preserves per-row accumulation order) and f16c dot kernels with scalar-order adds. - Per-step decode scalar inputs uploaded in 2 tensor_sets instead of ~430. Opt-in, default OFF (documented as not bit-identical): - DFLASH_DS4_FUSED_DECODE single-graph decode with GGML_OP_DS4_HC. - DFLASH_DS4_FFN_RAW_MMID / DFLASH_DS4_FFN_FUSED_COMBINE accumulation reorder. - DFLASH_DS4_ROCMFPX_HC_GPU GPU HC pre-mix.
Return confidence scores from the fused Markov graph in the existing token-id synchronization, then select q=2/q=3/q=4 from cumulative prefix confidence. Compatible DSpark artifacts enable the policy automatically; artifacts without a confidence head retain the existing EWMA controller. No new deployment configuration is introduced.\n\nValidated on gfx1151: GSM+Math 10/10 at 29.25 tok/s weighted, within 0.8% of fixed q=4, with adaptive behavior retained on low-acceptance prompts.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 48 files
Not reviewed (too large): server/src/deepseek4/deepseek4_graph.cpp (~2,569 lines), server/deps/llama.cpp/ggml/src/ggml-cpu/ops.cpp (~78 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
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/deps/llama.cpp/ggml/src/CMakeLists.txt">
<violation number="1" location="server/deps/llama.cpp/ggml/src/CMakeLists.txt:211">
P2: The `rocmfpx` sources are being added to `ggml-base`, but the companion test file `ggml/rocmfpx/test_rocmfpx.c` is not wired into any CMake target or CTest suite. That leaves the test uncompiled and unrunnable in CI, so regressions in the ROCmFP2/FP3/FP6/FP8 quantization paths could go undetected. Consider adding an `add_executable` and `add_test` target for `test_rocmfpx.c` so it builds and runs alongside the rest of the test suite.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ../rocmfp4/rocmfp4.c | ||
| ../rocmfp4/rocmfp4.h | ||
| ../rocmfpx/rocmfpx.c | ||
| ../rocmfpx/rocmfpx.h |
Contributor
There was a problem hiding this comment.
P2: The rocmfpx sources are being added to ggml-base, but the companion test file ggml/rocmfpx/test_rocmfpx.c is not wired into any CMake target or CTest suite. That leaves the test uncompiled and unrunnable in CI, so regressions in the ROCmFP2/FP3/FP6/FP8 quantization paths could go undetected. Consider adding an add_executable and add_test target for test_rocmfpx.c so it builds and runs alongside the rest of the test suite.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/deps/llama.cpp/ggml/src/CMakeLists.txt, line 211:
<comment>The `rocmfpx` sources are being added to `ggml-base`, but the companion test file `ggml/rocmfpx/test_rocmfpx.c` is not wired into any CMake target or CTest suite. That leaves the test uncompiled and unrunnable in CI, so regressions in the ROCmFP2/FP3/FP6/FP8 quantization paths could go undetected. Consider adding an `add_executable` and `add_test` target for `test_rocmfpx.c` so it builds and runs alongside the rest of the test suite.</comment>
<file context>
@@ -205,6 +205,10 @@ add_library(ggml-base
+ ../rocmfp4/rocmfp4.c
+ ../rocmfp4/rocmfp4.h
+ ../rocmfpx/rocmfpx.c
+ ../rocmfpx/rocmfpx.h
ggml-quants.h
gguf.cpp)
</file context>
Contributor
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
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/deps/llama.cpp/ggml/tests/CMakeLists.txt">
<violation number="1" location="server/deps/llama.cpp/ggml/tests/CMakeLists.txt:2">
P1: The test target `test-rocmfpx` will fail to compile because the source file `test_rocmfpx.c` includes `"rocmfp4.h"`, but the CMake target only exposes `ggml/include` and `ggml/rocmfpx` as include directories. The header lives in the sibling `ggml/rocmfp4/` directory, which is not in the include path. Any build with `GGML_BUILD_TESTS=ON` will produce a missing-header error. Add `${CMAKE_CURRENT_SOURCE_DIR}/../rocmfp4` to the `target_include_directories` call, or if the include is unused (the test does not reference any ROCmFP4-exclusive declarations), remove the `#include "rocmfp4.h"` line from the test source.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -0,0 +1,11 @@ | |||
| add_executable(test-rocmfpx ../rocmfpx/test_rocmfpx.c) | |||
| target_include_directories(test-rocmfpx PRIVATE | |||
Contributor
There was a problem hiding this comment.
P1: The test target test-rocmfpx will fail to compile because the source file test_rocmfpx.c includes "rocmfp4.h", but the CMake target only exposes ggml/include and ggml/rocmfpx as include directories. The header lives in the sibling ggml/rocmfp4/ directory, which is not in the include path. Any build with GGML_BUILD_TESTS=ON will produce a missing-header error. Add ${CMAKE_CURRENT_SOURCE_DIR}/../rocmfp4 to the target_include_directories call, or if the include is unused (the test does not reference any ROCmFP4-exclusive declarations), remove the #include "rocmfp4.h" line from the test source.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/deps/llama.cpp/ggml/tests/CMakeLists.txt, line 2:
<comment>The test target `test-rocmfpx` will fail to compile because the source file `test_rocmfpx.c` includes `"rocmfp4.h"`, but the CMake target only exposes `ggml/include` and `ggml/rocmfpx` as include directories. The header lives in the sibling `ggml/rocmfp4/` directory, which is not in the include path. Any build with `GGML_BUILD_TESTS=ON` will produce a missing-header error. Add `${CMAKE_CURRENT_SOURCE_DIR}/../rocmfp4` to the `target_include_directories` call, or if the include is unused (the test does not reference any ROCmFP4-exclusive declarations), remove the `#include "rocmfp4.h"` line from the test source.</comment>
<file context>
@@ -0,0 +1,11 @@
+add_executable(test-rocmfpx ../rocmfpx/test_rocmfpx.c)
+target_include_directories(test-rocmfpx PRIVATE
+ ${CMAKE_CURRENT_SOURCE_DIR}/../include
+ ${CMAKE_CURRENT_SOURCE_DIR}/../rocmfpx)
</file context>
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
2 participants
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.
Summary
Enable causal Flash prefill for DeepSeek4 ROCmFP2. The implementation defaults to an internal prefill batch of 128 tokens, the best passing prefill result from the sweep and workload comparison below; deployments can reduce it when tuning.
Stack:
main -> #502 -> #503 -> #496 -> this.Changes
DFLASH_DS4_FLASH_PREFILL_CHUNKcan reduce it.Validation
Validation used
DeepSeek-V4-Flash-ROCMFP2-STRIX.ggufon with ROCm 7.2.4, gfx1151,max_ctx=1048576,chunk=512, and fused decode.The same model, GPU target, server arguments, and prompts are used for before (baseline), 64-token and 128-token comparison. The different workload shows how different prompt impact the prefill speed.
hellotexthellotextThe 4k throughput prompt repeats a small token pattern and concentrates MoE routing. Prose, mathematics, and code distribute tokens across more experts, which adds ROCmFP2 dequantization and routed-FFN work not accelerated by Flash attention; this accounts for their lower prefill rates than 60.45 tok/s.
All prose, mathematics, and code responses remained in the requested content type and contained no malformed text. Their generation cap was 128 tokens.