[#16330][fix] Gate PDL on supported GPU architectures#16332
[#16330][fix] Gate PDL on supported GPU architectures#16332ProvenceLavanda wants to merge 2 commits into
Conversation
Signed-off-by: baiqiangfu <baiqiangfu@gmail.com>
📝 WalkthroughWalkthroughPDL enablement now requires ChangesPDL hardware gating
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/unittest/_torch/test_flashinfer_utils.py (2)
33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd type annotations to the test signature.
Annotate
monkeypatch,env_value,sm_version,expected, and the-> Nonereturn type.As per coding guidelines, all Python functions should be annotated; Python 3.10+ union syntax is supported in this repository.
🤖 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 `@tests/unittest/_torch/test_flashinfer_utils.py` at line 33, Annotate the test_get_env_enable_pdl_hardware_gate signature with types for monkeypatch, env_value, sm_version, and expected, and add a -> None return annotation. Use the repository’s Python 3.10+ union syntax where a parameter accepts multiple types.Sources: Coding guidelines, Learnings
21-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the new one-time logging behavior.
tests/unittest/_torch/test_flashinfer_utils.pysufficiently covers the boolean gate matrix, but the changed logging branch intensorrt_llm/_torch/flashinfer_utils.py(Lines 33-39) is untested. Add assertions for the enabled and SM89-disabled messages, including a second call to verify_printedsuppresses duplicates; otherwise track this as follow-up outside the PR.As per path instructions, boolean coverage is sufficient but logging coverage needs follow-up in
tests/unittest/_torch/test_flashinfer_utils.py.Also applies to: 34-41
🤖 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 `@tests/unittest/_torch/test_flashinfer_utils.py` around lines 21 - 32, Extend the parameterized tests for the flashinfer gate to cover logging in the changed branch of is_flashinfer_available, asserting the enabled message and the SM89-disabled message. Invoke the gate twice per case and verify the _printed state suppresses duplicate logs, while preserving the existing boolean matrix coverage.Source: Path instructions
🤖 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.
Nitpick comments:
In `@tests/unittest/_torch/test_flashinfer_utils.py`:
- Line 33: Annotate the test_get_env_enable_pdl_hardware_gate signature with
types for monkeypatch, env_value, sm_version, and expected, and add a -> None
return annotation. Use the repository’s Python 3.10+ union syntax where a
parameter accepts multiple types.
- Around line 21-32: Extend the parameterized tests for the flashinfer gate to
cover logging in the changed branch of is_flashinfer_available, asserting the
enabled message and the SM89-disabled message. Invoke the gate twice per case
and verify the _printed state suppresses duplicate logs, while preserving the
existing boolean matrix coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fb18be4b-75e7-4795-bc71-53e1c6f88675
📒 Files selected for processing (2)
tensorrt_llm/_torch/flashinfer_utils.pytests/unittest/_torch/test_flashinfer_utils.py
Description
TRTLLM_ENABLE_PDLdefaults to enabled, but the shared FlashInfer helper didnot validate GPU architecture before forwarding the value to FlashInfer
operators.
On SM89 GPUs, this can make FlashInfer's CuTe DSL RMSNorm path emit the
SM90+-only
griddepcontrol.*PDL instructions while compiling ansm_89target.
ptxascorrectly rejects that PTX and executor initialization fails.This PR gates PDL in
get_env_enable_pdl()on both the existing environmentrequest and
get_sm_version() >= 90.The change preserves:
TRTLLM_ENABLE_PDL=0;No C++ code, dependency version, or public LLM API is changed.
Fixes #16330 .
Test Coverage
TRTLLM_ENABLE_PDL=1both resolve toFalse;True;TRTLLM_ENABLE_PDL=0resolves toFalse.python3 -m pytest -q tests/unittest/_torch/test_flashinfer_utils.py7 passedpython3 -m pytest -q tests/unittest/_torch/attention/test_flashinfer_attention.py7 passedpre-commit run --files tensorrt_llm/_torch/flashinfer_utils.py tests/unittest/_torch/test_flashinfer_utils.pyQwen/Qwen2.5-7B-Instruct:PDL disabled on SM89: requires SM90 or newer;trtllm-servestarts successfully;GET /healthreturns HTTP 200;GET /v1/modelsreturnsQwen2.5-7B-Instruct.PR Checklist
api-compatibleorapi-breakinglabel is needed.Summary by CodeRabbit
Bug Fixes
Tests