[https://nvbugs/6071081][fix] Fix exception in checkIfKernelExist when kernel param is illegal#13019
[https://nvbugs/6071081][fix] Fix exception in checkIfKernelExist when kernel param is illegal#13019pengbowang-nv wants to merge 2 commits intoNVIDIA:mainfrom
Conversation
|
/bot run --add-multi-gpu-test --gpu-type "B200_PCIe,DGX_B200,B300,DGX_B300,GB200" |
|
PR_Github #43161 [ run ] triggered by Bot. Commit: |
|
/bot kill |
|
PR_Github #43172 [ kill ] triggered by Bot. Commit: |
|
PR_Github #43172 [ kill ] completed with state |
|
/bot run --add-multi-gpu-test --gpu-type "B200_PCIe,DGX_B200,B300,DGX_B300,GB200" --disable-fail-fast |
|
PR_Github #43393 [ run ] triggered by Bot. Commit: |
|
/bot kill |
|
/bot run --add-multi-gpu-test --gpu-type "B200_PCIe,DGX_B200,B300,DGX_B300,GB200" --disable-fail-fast |
|
PR_Github #43461 [ kill ] triggered by Bot. Commit: |
|
PR_Github #43461 [ kill ] completed with state |
|
PR_Github #43463 [ run ] triggered by Bot. Commit: |
|
PR_Github #43463 [ run ] completed with state
|
43da3d4 to
f4e84a9
Compare
|
/bot run --add-multi-gpu-test |
|
PR_Github #43641 [ run ] triggered by Bot. Commit: |
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h (1)
2-2:⚠️ Potential issue | 🟡 MinorUpdate the NVIDIA copyright year for this modified file.
Line 2 still ends at
2025, but this file has meaningful modifications in 2026.Suggested patch
- * Copyright (c) 2020-2025, NVIDIA CORPORATION. All rights reserved. + * Copyright (c) 2020-2026, NVIDIA CORPORATION. All rights reserved.As per coding guidelines: “Add NVIDIA copyright header on ALL new files and update year on modified files” and “All TensorRT-LLM source files must contain an NVIDIA copyright header with the year of latest meaningful modification.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h` at line 2, Update the NVIDIA copyright header in the modified header file fmhaKernels.h to reflect the latest meaningful modification year (change "2020-2025" to "2020-2026"); locate the copyright comment at the top of fmhaKernels.h and update the year range so the file header complies with the project guideline requiring the year of latest meaningful modification.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h`:
- Around line 244-248: The catch block that currently builds errorInfo as a
constant string loses the exception details; change the construction of
errorInfo to include e.what(), log it via TLLM_LOG_WARNING with the combined
message, and return the new errorInfo in the std::make_pair(false, errorInfo) so
callers receive the exception text (update the path that catches std::exception
const& e and the use of TLLM_LOG_WARNING / std::make_pair to reference the new
errorInfo).
---
Outside diff comments:
In `@cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h`:
- Line 2: Update the NVIDIA copyright header in the modified header file
fmhaKernels.h to reflect the latest meaningful modification year (change
"2020-2025" to "2020-2026"); locate the copyright comment at the top of
fmhaKernels.h and update the year range so the file header complies with the
project guideline requiring the year of latest meaningful modification.
🪄 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: Pro Plus
Run ID: 6f9304e6-ef23-4b37-91c3-a021e4ad4861
📒 Files selected for processing (1)
cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h
|
PR_Github #43641 [ run ] completed with state
|
|
/bot run --add-multi-gpu-test |
|
PR_Github #43751 [ run ] triggered by Bot. Commit: |
|
PR_Github #43751 [ run ] completed with state
|
Signed-off-by: Pengbo Wang <221450789+pengbowang-nv@users.noreply.github.com>
…nction Signed-off-by: Pengbo Wang <221450789+pengbowang-nv@users.noreply.github.com>
1d060ec to
0f391d0
Compare
|
/bot run --add-multi-gpu-test --disable-fail-fast |
|
PR_Github #43809 [ run ] triggered by Bot. Commit: |
Summary by CodeRabbit
Description
checkIfKernelExistmay get illegal kernel param during warmup. This will lead to a exception being raised and TRTLLM abort. The PR wrap it with try-catch so that a missing kernel/wrong input won't lead to TRTLLM abort.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)
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.