[TRTLLM-12507][feat] Cudagraph support for per-expert lora in Cutlass backend - Part 2 - #14881
Conversation
80bbb1e to
f526997
Compare
…h Cutlass backend Squashed combination of the full NVIDIA#14881 work (15 commits) prior to rebase onto main. Part 1 (NVIDIA#14923) has already been merged into main. Signed-off-by: Balaram Buddharaju <169953907+brb-nv@users.noreply.github.com>
b8aeac1 to
5069057
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #53103 [ run ] triggered by Bot. Commit: |
5069057 to
9a47f95
Compare
📝 WalkthroughWalkthroughThis PR adds slot-indexed tensor routing for routed-expert MoE LoRA within CUDA graphs. Per-token adapter selection is now driven by slot indices and slot-table lookups (instead of per-request expansion), enabling capture-safe replay with updated slot assignments. The implementation spans a new CUDA slot-expansion kernel, Python LoRA infrastructure for slot-table management, custom op extensions, C++ runner integration with buffer pre-reservation, and comprehensive tests. ChangesMoE LoRA Slot-Indexed CUDA-Graph Expansion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tensorrt_llm/_torch/modules/fused_moe/fused_moe_cutlass.py (1)
485-496:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTeach
_moe_lora_active()about slot-indexed CUDA-graph inputs.In CUDA-graph mode
lora_paramsonly carriescuda_graph_params, so this helper always returnsFalse. That bypasses both safety checks inforward_impl(), including the multi-chunk rejection at Lines 1428-1437. A chunked CUDA-graph MoE LoRA call will therefore reuse the full-batch slot tables for every chunk instead of failing fast.Suggested fix
def _moe_lora_active(self, lora_params: Optional[Dict]) -> bool: """Return True when lora_params carries routed-expert MoE LoRA tensors for this layer, meaning run_moe would fuse a LoRA delta. """ if not lora_params or self.layer_idx is None: return False + if lora_params.get("use_cuda_graph_mode", False): + cuda_graph_params = lora_params.get("cuda_graph_params") + if cuda_graph_params is None: + return False + return any( + cuda_graph_params.get_moe_slot_inputs(self.layer_idx, int(module_type)) + is not None + for module_type in ( + LoraModuleType.MOE_H_TO_4H, + LoraModuleType.MOE_4H_TO_H, + LoraModuleType.MOE_GATE, + ) + ) layer_params = lora_params.get(self.layer_idx, {}) if not layer_params: return False return any( int(LoraModuleType.from_string(name)) in layer_params🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tensorrt_llm/_torch/modules/fused_moe/fused_moe_cutlass.py` around lines 485 - 496, _moe_lora_active currently ignores cuda-graph slot-indexed inputs because in CUDA-graph mode lora_params carries its data under the "cuda_graph_params" envelope; update _moe_lora_active to unwrap cuda_graph_params when present (e.g., check if "cuda_graph_params" in lora_params and then use that dict) and then inspect the per-layer entry for the presence of any MoE LoRA module names from self._MOE_LORA_MODULE_NAMES by converting names via LoraModuleType.from_string and checking membership in the layer's keys; ensure the logic handles both the normal and slot-indexed shapes (nested dicts) so the function returns True when any routed-expert MoE LoRA tensor exists for self.layer_idx.cpp/tensorrt_llm/thop/moeOp.cpp (2)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the copyright year for this 2026 modification.
This file was meaningfully modified in 2026, but the header still ends at 2025. Please bump the NVIDIA copyright year to keep the header compliant.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/tensorrt_llm/thop/moeOp.cpp` at line 2, Update the copyright header year from "2022-2025" to "2022-2026" in the file's top-of-file header comment (the copyright line present in moeOp.cpp) so the header reflects the 2026 modification; ensure formatting and punctuation remain unchanged aside from the year range.Source: Coding guidelines
1412-1468:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe post-capture reallocation guard is not actually latched.
mLoraCaptureObservedis only set insidecheckLoraReallocSafeDuringCapture(), which means a graph captured with already-sized buffers leaves the flag false. After that, a laterreserve_lora_host_buffers()or larger eager request can still reallocate the pinned/device buffers whose addresses were baked into the captured graph. Please mark capture as soon as any LoRA call runs underisCapturing(stream)and route all host/slot/device growth through the same guard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/tensorrt_llm/thop/moeOp.cpp` around lines 1412 - 1468, The realloc guard flag mLoraCaptureObserved is only set inside checkLoraReallocSafeDuringCapture(), so growth paths can still reallocate after a graph has been captured; fix by detecting capture earlier and routing all buffer-growth through the same guard: in reserveLoraHostBuffers (and any public growth entrypoints that call ensureLoraExpandBuffers, ensureLoraSlotTableBuffers or ensureLoraDeviceScratch) acquire the current CUDA stream (or add a cudaStream_t parameter), call checkLoraReallocSafeDuringCapture(stream, requested, current) before any ensure* call to both set mLoraCaptureObserved when capturing and to reject unsafe growth, and ensure the same check is invoked inside ensureLoraExpandBuffers/ensureLoraSlotTableBuffers/ensureLoraDeviceScratch if they can be called independently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/tensorrt_llm/thop/moeOp.cpp`:
- Around line 1808-1852: The code performs cudaMemcpyAsync from caller CPU
tensors (token_to_slot, fc1_slot_lora_ranks, fc1_slot_lora_weight_ptrs,
fc2_slot_lora_ranks, fc2_slot_lora_weight_ptrs) which must be pinned for safe
CUDA-graph capture; add checks (e.g. TORCH_CHECK(tensor.is_pinned(), "... must
be pinned host tensors for captured async H2D")) for each of those tensors
before the h2d_slot lambda/copies (or wrap them in a helper that validates
is_pinned()) and fail early with a clear error message identifying the offending
tensor name so callers know to provide pinned memory.
- Around line 1817-1859: The code copies fc2 and gated slot tables to device
without validating their shapes causing possible OOB reads; before the h2d_slot
calls add the same TORCH_CHECKs used for fc1 (validate fc2_slot_lora_weight_ptrs
has dim()==2 and size == [num_slots,3] and fc2_slot_lora_ranks
size(0)==num_slots) and, when has_gated is true, validate
gated_slot_lora_weight_ptrs and gated_slot_lora_ranks similarly (use
CHECK_CPU_INPUT where appropriate), then proceed to call h2d_slot and the gated
copies; reference fc2_slot_lora_weight_ptrs, fc2_slot_lora_ranks,
gated_slot_lora_weight_ptrs, gated_slot_lora_ranks, num_slots, and h2d_slot.
In `@docs/source/features/lora.md`:
- Line 149: Update the LoRA docs to reflect the actual MoE adapter requirements
and CUDA-graph behavior: change the statement that “moe_gate and moe_4h_to_h are
required” to require both moe_h_to_4h and moe_4h_to_h (with moe_gate optional)
consistent with fused_moe_cutlass.py and
tests/unittest/_torch/lora/test_moe_lora_extract.py, and revise the CUDA-graph
section to note that CUDA-graph capture/support exists for routed-expert MoE
LoRA (referencing use_cuda_graph_mode, CudaGraphLoraManager and
tests/unittest/_torch/lora/test_moe_lora_device_path.py) rather than saying
CUDA-graph is entirely rejected for MoE LoRA.
In `@tensorrt_llm/_torch/custom_ops/torch_custom_ops.py`:
- Around line 315-326: The current checks only look at
fc1_lora_ranks/fc1_slot_lora_ranks; update the logic to detect any per-request
LoRA tensor present and any slot-indexed LoRA tensor present instead of keying
only on fc1_. Concretely, replace the lora_active bool and the mutual-exclusion
check with predicates that OR together all per-request LoRA symbols (e.g.,
fc1_lora_ranks, fc2_lora_ranks, ... any fc*_lora_* per-request tensors) and
separately OR together all slot-indexed symbols (e.g., fc1_slot_lora_ranks,
fc2_slot_lora_ranks, token_to_slot, ... any fc*_slot_lora_* / slot-related
tensors), then: (1) if min_latency_mode and either predicate is true raise the
same RuntimeError, and (2) if both predicates are true raise the
mutual-exclusion RuntimeError; use the existing error messages and reference the
same symbols (fc1_lora_ranks, fc1_slot_lora_ranks, token_to_slot) in the code to
locate where to change.
In `@tensorrt_llm/_torch/modules/fused_moe/fused_moe_cutlass.py`:
- Around line 634-642: The code currently returns None when either
slot_ranks["fc1"] or slot_ranks["fc2"] is missing during CUDA-graph capture,
silently dropping partial MoE LoRA configs; change the logic after the loop over
slot_to_kernel to detect partial configurations and raise an explicit error
instead of returning None: use the same consistency check as the eager path (if
exactly one of slot_ranks["fc1"] or slot_ranks["fc2"] is None) then raise a
ValueError describing the partial MoE LoRA config for self.layer_idx, otherwise
(both None) allow the existing None return; update the block that references
slot_ranks/slot_ptrs and cuda_graph_params.get_moe_slot_inputs to implement this
check.
In `@tensorrt_llm/_torch/peft/lora/cuda_graph_lora_params.py`:
- Around line 225-233: Guard the runtime tokens_per_seq against the preallocated
host capacity: compute the host capacity (e.g., max_tokens =
self.token_to_slot_host.numel()), then clamp the effective per-sequence token
count (use effective_tokens_per_seq = min(tokens_per_seq, max_tokens //
batch_size_or_slot_count)) before building token_slots (repeat_interleave on
slot_ids_t) and adjust num_tokens accordingly so
self.token_to_slot_host[:num_tokens].copy_(token_slots) never tries to copy more
elements than the preallocated buffer; apply the same clamping logic
consistently where sorted_ids and the MoE workspace reservation are assumed
sized from max_batch_size * max_tokens_per_seq and optionally raise a clear
error if the requested runtime tokens exceed the preallocated budget.
---
Outside diff comments:
In `@cpp/tensorrt_llm/thop/moeOp.cpp`:
- Line 2: Update the copyright header year from "2022-2025" to "2022-2026" in
the file's top-of-file header comment (the copyright line present in moeOp.cpp)
so the header reflects the 2026 modification; ensure formatting and punctuation
remain unchanged aside from the year range.
- Around line 1412-1468: The realloc guard flag mLoraCaptureObserved is only set
inside checkLoraReallocSafeDuringCapture(), so growth paths can still reallocate
after a graph has been captured; fix by detecting capture earlier and routing
all buffer-growth through the same guard: in reserveLoraHostBuffers (and any
public growth entrypoints that call ensureLoraExpandBuffers,
ensureLoraSlotTableBuffers or ensureLoraDeviceScratch) acquire the current CUDA
stream (or add a cudaStream_t parameter), call
checkLoraReallocSafeDuringCapture(stream, requested, current) before any ensure*
call to both set mLoraCaptureObserved when capturing and to reject unsafe
growth, and ensure the same check is invoked inside
ensureLoraExpandBuffers/ensureLoraSlotTableBuffers/ensureLoraDeviceScratch if
they can be called independently.
In `@tensorrt_llm/_torch/modules/fused_moe/fused_moe_cutlass.py`:
- Around line 485-496: _moe_lora_active currently ignores cuda-graph
slot-indexed inputs because in CUDA-graph mode lora_params carries its data
under the "cuda_graph_params" envelope; update _moe_lora_active to unwrap
cuda_graph_params when present (e.g., check if "cuda_graph_params" in
lora_params and then use that dict) and then inspect the per-layer entry for the
presence of any MoE LoRA module names from self._MOE_LORA_MODULE_NAMES by
converting names via LoraModuleType.from_string and checking membership in the
layer's keys; ensure the logic handles both the normal and slot-indexed shapes
(nested dicts) so the function returns True when any routed-expert MoE LoRA
tensor exists for self.layer_idx.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3f4ef076-e048-4ea7-96f7-94c3253b5665
📒 Files selected for processing (15)
cpp/tensorrt_llm/kernels/cutlass_kernels/include/moe_lora_device_path.hcpp/tensorrt_llm/kernels/cutlass_kernels/include/moe_lora_slot_expand.hcpp/tensorrt_llm/kernels/cutlass_kernels/moe_gemm/moe_lora_slot_expand.cucpp/tensorrt_llm/thop/moeOp.cppcpp/tests/unit_tests/kernels/CMakeLists.txtcpp/tests/unit_tests/kernels/moeLoraSlotExpandTest.cudocs/source/features/lora.mdtensorrt_llm/_torch/custom_ops/torch_custom_ops.pytensorrt_llm/_torch/modules/fused_moe/fused_moe_cutlass.pytensorrt_llm/_torch/peft/lora/cuda_graph_lora_manager.pytensorrt_llm/_torch/peft/lora/cuda_graph_lora_params.pytensorrt_llm/_torch/peft/lora/layer.pytests/unittest/_torch/lora/test_moe_lora_cuda_graph_params.pytests/unittest/_torch/lora/test_moe_lora_device_path.pytests/unittest/_torch/lora/test_moe_lora_op.py
|
/bot run --disable-fail-fast |
|
PR_Github #53115 [ run ] triggered by Bot. Commit: |
|
PR_Github #53103 [ run ] completed with state |
fa6de78 to
f4687aa
Compare
…h Cutlass backend - Part 2 Signed-off-by: Balaram Buddharaju <169953907+brb-nv@users.noreply.github.com>
420cc70 to
5aa968c
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #53158 [ run ] triggered by Bot. Commit: |
|
PR_Github #53115 [ run ] completed with state |
|
PR_Github #53158 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #53342 [ run ] triggered by Bot. Commit: |
|
PR_Github #53342 [ run ] completed with state |
Description
This is Part 2 of routed-expert MoE LoRA on the CUTLASS backend, building on the device path merged in Part 1 (#14923). It makes the device path fully CUDA-graph capturable with per-request adapters under a single persistent graph:
moeOp.cpp: slot-indexed input schema, persistent device slot-table buffers sized via reserve_lora_host_buffers, captured async H2D from stable pinned buffers, and auto-enabling the device path for the slot-indexed schema.cuda_graph_lora_params.py,cuda_graph_lora_manager.py,fused_moe_cutlass.py,layer.py): produces the slot-indexedfused_moekwargs at decode time and packs the per-module pointer tables behind a fixed cache.Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests