fix(gemm): report a failed cuDNN tactic as unsupported during profiling (not silent tactic=-1) - #4132
YangXu1990uiuc wants to merge 1 commit into
Conversation
…ng, not silent -1 The cuDNN GEMM runners (FP8/BF16/FP4/MXFP8/BF16-FP4) catch a failed or unresolvable tactic and silently execute tactic=-1, both when resolving an engine/knob tactic to a plan index and when a tactic errors at execute. That is correct for serving robustness, but during autotuner profiling it attributes the fallback's timing to the requested tactic -- so a tactic that never ran can win the autotune, and a genuinely faster tactic can be masked by the default's time. The tuner then persists a 'winner' that always falls back at serving, violating 'serve what was tuned'. Raised in the flashinfer-ai#3707 review and deferred ('would require a relatively broad autotuner change'). It does not: the profiling scope is already detectable via is_in_profile_measurement() (from flashinfer-ai#3252), and choose_one already marks a raised tactic as inf and disqualifies it. So each fallback site just re-raises during profiling and keeps the warn+fallback for serving. 8 sites: 2 in the tactic->plan_index resolver (no-match / out-of-range) + 6 runner-forward except clauses. GPU test (SM100): an out-of-range tactic warns+falls-back+produces valid output outside profiling, and raises (disqualified) inside a profile-measurement scope. AI-assisted (Claude Code). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughcuDNN GEMM tactic resolution and runner failures now raise during AutoTuner profiling, while serving continues warning and falling back to ChangescuDNN profiling failure handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AutoTuner
participant CudnnFp8GemmRunner
participant cuDNN
AutoTuner->>CudnnFp8GemmRunner: execute requested tactic
CudnnFp8GemmRunner->>cuDNN: resolve and run tactic plan
cuDNN-->>CudnnFp8GemmRunner: tactic failure
alt Profiling
CudnnFp8GemmRunner-->>AutoTuner: re-raise RuntimeError
else Serving
CudnnFp8GemmRunner-->>AutoTuner: warn and use tactic=-1
end
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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/autotuner/test_cudnn_profiling_fallback.py`:
- Around line 26-27: Update the output allocation in the test setup around the
existing out tensor to initialize every element to NaN instead of leaving it
uninitialized, while preserving its CUDA device and bfloat16 dtype. Apply the
same initialization to the additional output allocation noted in the comment so
the subsequent isfinite assertions reliably detect whether fallback populated
the results.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: efcbbcee-d85e-49c9-8a1b-23076b329c88
📒 Files selected for processing (3)
flashinfer/gemm/gemm_base.pyflashinfer/gemm/gemm_bf16_fp4_cudnn.pytests/autotuner/test_cudnn_profiling_fallback.py
| out = torch.empty([1, m, n], device="cuda", dtype=torch.bfloat16) | ||
| ws = torch.empty(32 * 1024 * 1024, device="cuda", dtype=torch.uint8) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Initialize the output with NaNs before asserting fallback output.
torch.empty may already contain finite values, allowing this assertion to pass if fallback never writes out. Initialize out to NaNs so isfinite() proves the fallback populated every result element.
Suggested fix
- out = torch.empty([1, m, n], device="cuda", dtype=torch.bfloat16)
+ out = torch.full(
+ [1, m, n], float("nan"), device="cuda", dtype=torch.bfloat16
+ )Also applies to: 54-55
🤖 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/autotuner/test_cudnn_profiling_fallback.py` around lines 26 - 27,
Update the output allocation in the test setup around the existing out tensor to
initialize every element to NaN instead of leaving it uninitialized, while
preserving its CUDA device and bfloat16 dtype. Apply the same initialization to
the additional output allocation noted in the comment so the subsequent isfinite
assertions reliably detect whether fallback populated the results.
📌 Description
The cuDNN GEMM runners (FP8 / BF16 / FP4 / MXFP8 / BF16×FP4) silently execute
tactic=-1when a requested tactic can't be used — both when resolving an engine/knob tactic to a plan index (no match / out-of-range) and when a tactic errors at execute time. That is correct for serving robustness, but during autotuner profiling it attributes the fallback's timing to the requested tactic. Consequences:validate_tactic-style checks can't catch it because the tactic "works" (via fallback).Fix
Report a failed tactic as unsupported during profiling (re-raise), so the autotuner marks it
infand disqualifies it; keep the warn-and-fall-back path for serving. The profiling scope is already detectable viais_in_profile_measurement(), andchoose_onealready turns a raised tactic intoinf+ skip — so this is a small per-site change, not an autotuner rework.8 sites: 2 in the tactic→plan_index resolver (
_get_cudnn_plan_index_for_tactic: no-match, out-of-range) + 6 runner-forwardexceptclauses (fp8/bf16/mxfp8×2/fp4 ingemm_base.py, bf16×fp4 ingemm_bf16_fp4_cudnn.py).🔍 Related Issues
Raised in the #3707 review ("Do not treat a failed tactic's fallback execution as a successful tactic during profiling") and deferred there — the concern at the time was that it "would require a relatively broad autotuner change." Re-examined:
is_in_profile_measurement()has existed since #3252 (before #3707), and the autotuner already marks a raised tacticinf, so the change is local to the runner fallback sites. Independently re-surfaced during a CUDA-graph-compatibility audit of the autotuner-v2 work (#3861).🧪 Tests
tests/autotuner/test_cudnn_profiling_fallback.py(GPU, SM90+): an out-of-range tactic on the real cuDNN FP8 runner warns + falls back + produces valid output outside profiling, and raises (so it's disqualified) inside a_profile_measurement_scope(). Verified on SM100. Serving behavior is unchanged.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests