[feat](kt-kernel): opt-in skip of CPU copies for GPU-resident experts#2085
[feat](kt-kernel): opt-in skip of CPU copies for GPU-resident experts#2085gvr13n wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the skip_gpu_expert_cpu_copy feature to optimize memory usage by skipping the allocation and copying of CPU weight buffers for GPU-resident experts. However, the code review highlights several critical issues: the feature is currently ignored in AMXMoEWrapper, making it ineffective for the default AMX backends, and multiple weight loading, saving, and writing paths in moe.hpp, awq-moe.hpp, mxfp8-moe.hpp, and fp4-moe.hpp lack proper guards or safety checks, which will lead to null pointer dereferences and segfaults when the feature is enabled.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if backend_cls is NativeMoEWrapper: | ||
| extra_kwargs["skip_gpu_expert_cpu_copy"] = skip_gpu_expert_cpu_copy |
There was a problem hiding this comment.
While skip_gpu_expert_cpu_copy is correctly passed to NativeMoEWrapper, it is completely ignored for AMXMoEWrapper (which handles AMXINT4 and AMXINT8 quantization methods).
In kt-kernel/python/utils/amx.py, AMXMoEWrapper.__init__ does not accept skip_gpu_expert_cpu_copy, and its load_weights / load_weights_from_tensors methods do not set moe_config.skip_gpu_expert_cpu_copy. This means the feature will have no effect when using the default/most common AMXINT4 or AMXINT8 backends, even though moe.hpp was updated to support it.
To fix this, please:
- Add
skip_gpu_expert_cpu_copy: bool = FalsetoAMXMoEWrapper.__init__and store it asself.skip_gpu_expert_cpu_copy. - Set
moe_config.skip_gpu_expert_cpu_copy = self.skip_gpu_expert_cpu_copyin bothload_weightsandload_weights_from_tensorsofAMXMoEWrapper. - Update
experts.pyto passskip_gpu_expert_cpu_copytoAMXMoEWrapperas well.
There was a problem hiding this comment.
Intentional scoping, not an oversight: the user-facing flag is resolved (sglang side, kvcache-ai/sglang#64) into an effective flag that is only true for mask-static paths — MXFP4, or MXFP8 with dynamic expert update off — which are NativeMoEWrapper paths. AMXINT4/8 can be demoted at runtime by the dynamic-update mask rewrite, so an expert whose CPU copy was skipped could later be needed on CPU; wiring the flag through AMXMoEWrapper without that guarantee would trade a safe no-op for a correctness hazard. If a mask-static AMX mode lands later, plumbing it through is a small follow-up.
| config_.expert_num, nullptr, | ||
| [this, physical_to_logical_map](int expert_id) { | ||
| // printf("Load layer %d [%d/%d]\n", config_.layer_idx, expert_id, config_.expert_num); | ||
| if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)expert_id)) return; |
There was a problem hiding this comment.
There are two other loader paths in moe.hpp that are missing the should_skip_expert guards and will SEGFAULT if skip_gpu_expert_cpu_copy is enabled:
-
The
config_.loadbranch (around line 256):
When loading pre-quantized weights from disk, the loop iterates over all experts without checking if they are skipped. Sinceup_bb_[expert_idx]isnullptrfor skipped experts, callingread_weightson it will dereferencenullptrand crash. -
The
config_.savebranch (around line 331):
When saving weights, the loop iterates over all experts and callswrite_weightswithup_bb_[expert_idx]->b, which will also dereferencenullptrand crash.
Please add the same should_skip_expert guards to both of these branches. For example, in the config_.save branch:
[this, physical_to_logical_map, prefix](int task_id) {
int64_t expert_idx = task_id / mat_type_all;
if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)expert_idx)) return;
expert_idx = expert_map(physical_to_logical_map, expert_idx);
...There was a problem hiding this comment.
Good catch — fixed in 54e0943: the config_.load work-stealing lambda now early-returns via should_skip_expert(expert_idx) (the same physical index used to access gate/up/down_bb_) before any buffer deref.
| nth * config_.expert_num, nullptr, | ||
| [this, nth, physical_to_logical_map](int task_id) { | ||
| uint64_t expert_idx = task_id / nth; | ||
| if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)expert_idx)) return; |
There was a problem hiding this comment.
The config_.save branches in awq-moe.hpp (around lines 583 and 631) are missing should_skip_expert guards.
When config_.save is enabled, the loops iterate over all experts from 0 to config_.expert_num and call write_weights with gate_bb_[expert_idx].get(). If skip_gpu_expert_cpu_copy is true, gate_bb_[expert_idx] is nullptr for skipped experts, causing a segfault when write_weights attempts to access buffer->b.
Please add a guard to skip saving weights for skipped experts:
if (config_.save) {
for (int expert_idx = 0; expert_idx < config_.expert_num; expert_idx++) {
if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)expert_idx)) continue;
write_weights(prefix, "_gate_", gate_bb_[expert_idx].get(), expert_idx, "OFFLINE");
...There was a problem hiding this comment.
Fixed in 54e0943: both config_.save loops in awq-moe.hpp (OFFLINE ~:583 and ONLINE ~:631) now continue on should_skip_expert(expert_idx).
| nth * config_.expert_num, nullptr, | ||
| [this, nth, physical_to_logical_map](int task_id) { | ||
| uint64_t expert_idx = task_id / nth; | ||
| if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)expert_idx)) return; |
There was a problem hiding this comment.
In mxfp8-moe.hpp (and similarly in fp4-moe.hpp), write_weights_to_buffer is used to copy CPU expert weights to the GPU pinned buffer.
If skip_gpu_expert_cpu_copy is enabled, the CPU buffers (gate_bb_, up_bb_, down_bb_) for GPU-resident experts are nullptr. If write_weights_to_buffer is called for a skipped expert, it will dereference nullptr (e.g., bb->b) and SEGFAULT.
To prevent silent crashes, please add a safety check at the beginning of write_weights_to_buffer in both mxfp8-moe.hpp and fp4-moe.hpp:
if (gate_bb_[expert_id] == nullptr) {
throw std::runtime_error("Cannot write weights to buffer for skipped/GPU-resident expert");
}There was a problem hiding this comment.
This path is safe by construction: write_weights_to_buffer is only reached from the sglang full-GPU-prefill pipeline, whose mask-aware callers (_prepare_weight_fp8/_prepare_weight_mxfp4) partition experts and submit ONLY cpu_expert_ids to the wrapper — a skipped (GPU-resident) expert never reaches it. Adding a null-check inside would silently mask a caller bug rather than surface it, so I have left it unguarded deliberately. Happy to add an assert instead if maintainers prefer.
In a hybrid GPU/CPU MoE split (--kt-num-gpu-experts N) kt-kernel loads every expert into the CPU AMX weight buffers, including the N that are uploaded to and computed on the GPU. Those experts are skipped on the CPU side via should_skip_expert(), so their host copies are never read again -- pure dead weight (~63GB of host RAM at N=110 for DeepSeek-V4-Flash, which is what pushed decode into swap on a 123GB box). Add an opt-in config, MOEConfig::skip_gpu_expert_cpu_copy, that never allocates or loads the CPU weight buffers for GPU-resident experts. The shared AMX_MOE_BASE::init() leaves a null BufferB at each masked slot -- the same state forward_*() already tolerates (m_local_num_ == 0) -- and every backend loader that inherits this base now skips those slots: fp4 (MXFP4), mxfp8 (MXFP8), the int4/int8/bf16 family, and awq. Each guard keys on the same index used to access the buffer, so a non-identity physical_to_logical_map stays correct. Off by default and resolved on the Python side, then passed down as a config bool (no new env vars). Adds a deterministic per_commit test that runs a hybrid split with and without the skip and asserts bitwise-identical output on both the decode and prefill paths, plus correctness against a torch reference that drops the masked experts.
c07a200 to
54e0943
Compare
|
I can see your point : for those experts already on gpu , the cpu side copies are unnecessary. I have 2 question:
|
|
Both cases are handled in the sglang integration side: kvcache-ai/sglang PR #64 — the kt-kernel 1. The skip is only activated when the GPU-expert set is provably static (kt_ep_wrapper.py#L2662-L2678): _mask_static = _method == "MXFP4" or (_method == "MXFP8" and not _dyn)
_kt_skip_cpu_copy = self.kt_config.kt_skip_gpu_expert_cpu_copy and _mask_static
2. Layerwise / full-GPU prefill It never reads GPU-resident experts from CPU RAM.
This combination (MXFP4, skip ON, |
Here is one more thing that I confirmed this afternoon: ktransformers's SFT part recently has shared same pipeline with inference path, for which a full weight on cpu ram is neccessary. Nice work as it is, it's a little pity that this PR will not be merged right away. (By the way, 350 tok/s is usually not the speed when you turned on layerwise prefill, especially for powerful machine like PRO 6000 with your description. But I believe your PR has no problem with that ! |
…the opt-in flag Three of the guards added for the offline-quantization save/load paths called should_skip_expert() unqualified. AMX_MOE_BASE is a dependent base, so any build that instantiates the AMX backends (Intel AMX targets, including the SFT bindings, whose TP_MOE_SFT::load_weights reaches AMX_MOE_TP::load_weights) fails to compile; my AMD build never instantiates them, which is how this slipped through. The same three guards also omitted the skip_gpu_expert_cpu_copy condition, which would have skipped GPU-masked experts on the offline cache save/load paths even with the flag off. Route all three through config_ and gate them on the flag, matching every other guard in the commit. Verified by explicit instantiation of TP_MOE_SFT<AMX_SFT_MOE_TP<BF16/Int8/Int4>>, AMX_MOE_TP<Int8/Int4> and AMX_AWQ_MOE_TP<Int4_1_LowKGroup> with g++ -fsyntax-only.
SFT trains every expert on CPU, so full host copies of all expert weights are mandatory, GPU-resident experts included. Today the flag provably cannot reach a training run (the KTMoEWrapper factory only plumbs it into the inference branch and the SFT loaders never read it), but MOESFTConfig inherits the field from GeneralMOEConfig and TP_MOE_SFT passes the sliced config into the shared TP_MOE base, so a future refactor of the shared loading pipeline could start honoring a mis-set flag silently. Close that at both layers: the Python factory raises ValueError when mode='sft' is combined with skip_gpu_expert_cpu_copy=True, and the TP_MOE_SFT constructor strips the inherited flag (with a warning) before the config reaches the shared base. Adds a per-commit test for the factory rejection.
|
@yyj6666667 - First off, thank you for taking the time to review this and for confirming the SFT detail. I get the maintenance cost: this is an opt-in path nobody on the team asked for, and every future loading-pipeline change would have to be checked against it. With SFT now sharing the pipeline with inference, and needing full weights in host RAM, deferring makes sense. No hard feelings. To make the branch safe to revisit later, I pushed a guard: the On the 350 tok/s: that is with layerwise prefill on (threshold 4096, chunk 8192). The bottleneck is the platform, not the GPU. It's a consumer desktop, Ryzen 9950X with dual-channel DDR5-5200 (~54 GB/s measured), streaming 156 of 256 experts per layer from host RAM each chunk, measured cold. On a real multi-channel server it should be several times faster. |
|
@yyj6666667 - You were right about the 350 tok/s, and thanks for the nudge. I dug in and it was not the layer-wise reload at all: on Constraining block_m / block_k / num_stages for large-M calls takes it from 342ms to 13ms per layer, and end-to-end cold prefill went from ~350 tok/s to ~910 tok/s on the same box. The weight reload is the bottleneck only now that the GEMM is fixed. Happy to send that as a separate PR if useful, since it should help any GPU outside the SM_100 whitelist. |
Implements the proposal in #2084 (follow-up to #2023).
With
--kt-num-gpu-experts N, kt-kernel still allocates and loads all experts per layer into host RAM, including the N that run on the GPU. Those CPU copies are dead weight:forward_*skips them viashould_skip_expert(), so they are filled at load and never read. At N=110 on DeepSeek-V4-Flash that is ~63GB of redundant host RAM; on a 123GB box it is the difference between fitting and swap-thrashing (scheduler RSS ~172GB → ~99GB, decode swap io → 0, identical output).Design (never-allocate, gated):
AMX_MOE_BASE::init()pushes nullgate_bb_/up_bb_/down_bb_placeholders for masked experts instead ofaligned_alloc— the same stateforward_*already tolerates (m_local_num_==0).init()guards withshould_skip_experton the same index it uses to access the buffers:fp4-moe.hpp,mxfp8-moe.hpp,moe.hpp(int4/int8/bf16),awq-moe.hpp. (mxfp8 previously had no guard and would have segfaulted on the null buffers — fixed here.)MOEConfig.skip_gpu_expert_cpu_copy(C++ bool, default false, pybinddef_readwrite). The sglang side (companion PR: feat(kt): add --kt-skip-gpu-expert-cpu-copy to reclaim host RAM for GPU experts sglang#64) resolves the user-facing flag into an effective flag that is only true when the path's full-GPU prefill fallback is mask-aware (MXFP4, or MXFP8 with dynamic expert update off); otherwise it warns once and falls back to stock behavior.Test:
test/per_commit/test_moe_amx_skip_gpu_expert_cpu_copy.py— deterministic (seed 1234), CPU-only, runs a hybrid split with agpu_experts_maskand asserts exact bitwise parity skip-on vs skip-off on both decode (qlen=1) and prefill (qlen=7), plus correctness vs a torch reference. Full per_commit suite passes (the pre-existing unseeded flake intest_moe_amx_accuracy_int4_1.pyis unrelated).Diff is pure additions (+221, 10 files), rebased on current main.