[https://nvbugs/6424956][fix] Support large FP8 quantization grids - #16933
Conversation
Signed-off-by: Fanrong Li <lfr-0531@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #62201 [ run ] triggered by Bot. Commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughFP8 packed quantization now distributes row blocks across CUDA ChangesFP8 quantization grid scaling
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant QuantizationLaunch
participant makeQuantizeGrid
participant CUDAKernel
QuantizationLaunch->>makeQuantizeGrid: Pass num_packed_sf_k and m_blocks
makeQuantizeGrid-->>QuantizationLaunch: Return 3D CUDA grid
QuantizationLaunch->>CUDAKernel: Launch with grid.y and grid.z
CUDAKernel->>CUDAKernel: Combine blockIdx.z and blockIdx.y into mBlockIdx
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/unittest/_torch/thop/parallel/test_fp8_quantize.py`:
- Line 400: Add an equivalent pytest parameter for the grid-y overflow boundary
case to the test coverage invoking launch_fp8_quantize_1x128_cutedsl_bf16_e4m3,
matching the existing packed-path inputs and expected behavior while retaining
the descriptive grid-y-overflow identifier.
🪄 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: 4e98a3c0-1adb-400c-8297-a713a6aee6ab
📒 Files selected for processing (2)
cpp/tensorrt_llm/kernels/cutlass_kernels/fp8_blockscale_gemm/fp8_blockscale_quant_packed.cutests/unittest/_torch/thop/parallel/test_fp8_quantize.py
|
PR_Github #62201 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #62350 [ run ] triggered by Bot. Commit: |
|
PR_Github #62350 [ run ] completed with state |
jiaganc
left a comment
There was a problem hiding this comment.
Reproduce script passed. LGTM.
Compute the linearized row block and row index in 64-bit arithmetic so padded blocks in large two-dimensional grids cannot overflow signed int indexing. Signed-off-by: Fanrong Li <lfr-0531@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #62493 [ run ] triggered by Bot. Commit: |
|
PR_Github #62493 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #62544 [ run ] triggered by Bot. Commit: |
|
PR_Github #62544 [ run ] completed with state |
Dev Engineer Review
grid.yandgrid.z, preventing failures whenmax_num_tokenspushesgrid.ybeyond CUDA’s 65,535 dimension limit.blockIdx.zandblockIdx.yinto the FP8 row-block index (m_idx), while preserving the prior mapping behavior for launches within the previous limit (e.g.,grid.z == 1).makeQuantizeGrid(numPackedSfK, mBlocks)to construct the bounded 3D grid rather than hardcodingdim3(numPackedSfK, m_blocks, 1).QA Engineer Review
fp8_quantize_1x128_packed_ue8m0_matches_legacyparameterization with a boundary casem=262141, k=128(id="grid-y-overflow") to exercise the grid-dimension limit fix.tests/integration/test_lists/test-db,tests/integration/test_lists/qa, ortests/integration/test_lists/*.txt).Description
NVBug 6424956 reports a
cudaLaunchKernelExinvalid-argument failure whenmax_num_tokensis 64K. The fused 1x128 FP8 quantization kernels mapped allrow blocks to
grid.y; after token expansion, an aligned row count of 262144requires 65536 row blocks, exceeding CUDA's 65535 limit for
grid.y.This change distributes row blocks across
grid.yandgrid.zand linearizesthe two indices in the kernel. Launches within the existing limit retain
grid.z == 1and the same row mapping. A boundary regression case comparesthe fused path with the legacy quantize-and-pack path.
There are no API or dependency changes. The expected functional impact is
limited to enabling large FP8 quantization launches that previously failed.
GPU execution remains unverified because the available B200 node's NVLink
Fabric stayed in
In Progressstate and CUDA initialization returned error802; the PR is therefore submitted as a draft.
Related: NVBug 6424956, TRTLLM-14125.
Test Coverage
tensorrt_llm,th_common, and Python bindings for SM100.libth_common.soand confirmed the target op isregistered and all mapped TensorRT-LLM libraries come from this
worktree.
SR_CTAID.Zwithcuobjdump.test_fp8_quantize.py::test_fp8_quantize_1x128_packed_ue8m0_matches_legacy[grid-y-overflow]on B200. This is blocked by the node-level NVLink Fabric/CUDA error 802
described above.
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.