Skip to content

[#16330][fix] Gate PDL on supported GPU architectures#16332

Open
ProvenceLavanda wants to merge 2 commits into
NVIDIA:mainfrom
ProvenceLavanda:fix/pdl-sm89-hardware-gate
Open

[#16330][fix] Gate PDL on supported GPU architectures#16332
ProvenceLavanda wants to merge 2 commits into
NVIDIA:mainfrom
ProvenceLavanda:fix/pdl-sm89-hardware-gate

Conversation

@ProvenceLavanda

@ProvenceLavanda ProvenceLavanda commented Jul 13, 2026

Copy link
Copy Markdown

Description

TRTLLM_ENABLE_PDL defaults to enabled, but the shared FlashInfer helper did
not 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 an sm_89
target. ptxas correctly rejects that PTX and executor initialization fails.

This PR gates PDL in get_env_enable_pdl() on both the existing environment
request and get_sm_version() >= 90.

The change preserves:

  • default PDL enablement on SM90+;
  • explicit disablement through TRTLLM_ENABLE_PDL=0;
  • existing public APIs and model configuration.

No C++ code, dependency version, or public LLM API is changed.

Fixes #16330 .

Test Coverage

  • Added a CPU-only unit test for the PDL decision matrix:
    • SM89: default and explicit TRTLLM_ENABLE_PDL=1 both resolve to False;
    • SM90 and SM100: default and explicit enablement resolve to True;
    • explicit TRTLLM_ENABLE_PDL=0 resolves to False.
  • python3 -m pytest -q tests/unittest/_torch/test_flashinfer_utils.py
    • 7 passed
  • python3 -m pytest -q tests/unittest/_torch/attention/test_flashinfer_attention.py
    • 7 passed
  • pre-commit run --files tensorrt_llm/_torch/flashinfer_utils.py tests/unittest/_torch/test_flashinfer_utils.py
    • passed
  • Manual L20 / SM89 end-to-end validation with Qwen/Qwen2.5-7B-Instruct:
    • default PDL configuration logs:
      PDL disabled on SM89: requires SM90 or newer;
    • trtllm-serve starts successfully;
    • GET /health returns HTTP 200;
    • GET /v1/models returns Qwen2.5-7B-Instruct.

PR Checklist

  • PR description explains the issue and fix.
  • Change follows TensorRT-LLM coding guidelines.
  • Regression tests cover the new hardware gate.
  • No public API change; no api-compatible or api-breaking label is needed.
  • No dependency, CODEOWNERS, documentation, or architecture-diagram change is required.
  • Appropriate PyTorch backend reviewers are assigned automatically or manually.

Summary by CodeRabbit

  • Bug Fixes

    • Improved PDL enablement checks to respect both the configuration setting and GPU compatibility.
    • PDL now activates only on supported SM90-or-newer GPUs and reports its status clearly.
  • Tests

    • Added coverage for PDL behavior across configuration values and GPU architectures.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

PDL enablement now requires TRTLLM_ENABLE_PDL="1" and GPU SM90 or newer. The helper logs the resulting decision once, and a parametrized test covers environment and SM-version combinations.

Changes

PDL hardware gating

Layer / File(s) Summary
SM-aware PDL decision and validation
tensorrt_llm/_torch/flashinfer_utils.py, tests/unittest/_torch/test_flashinfer_utils.py
get_env_enable_pdl() combines the environment flag with an SM90+ requirement, logs enabled or disabled outcomes once, and is tested across supported and unsupported GPU versions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: byshiue, zhaoyangwang-nvidia

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the PR’s main change: gating PDL on supported GPU architectures.
Description check ✅ Passed The description includes the issue, fix, test coverage, and checklist items required by the template.
Linked Issues check ✅ Passed The code and tests satisfy #16330 by disabling PDL on SM89 and preserving enablement on SM90+.
Out of Scope Changes check ✅ Passed The PR stays focused on the PDL hardware gate and its tests, with no unrelated code changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
tests/unittest/_torch/test_flashinfer_utils.py (2)

33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add type annotations to the test signature.

Annotate monkeypatch, env_value, sm_version, expected, and the -> None return 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 win

Cover the new one-time logging behavior.

tests/unittest/_torch/test_flashinfer_utils.py sufficiently covers the boolean gate matrix, but the changed logging branch in tensorrt_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 _printed suppresses 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

📥 Commits

Reviewing files that changed from the base of the PR and between 50142a9 and b1941e0.

📒 Files selected for processing (2)
  • tensorrt_llm/_torch/flashinfer_utils.py
  • tests/unittest/_torch/test_flashinfer_utils.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Default PDL enablement fails FlashInfer RMSNorm JIT on SM89 GPUs

1 participant