Repository navigation
Conversation
TRT-LLM PR #12015 (merged 2026-03-11) wrapped NCCLWindowAllocator and createNCCLWindowTensor behind #if NCCL_VERSION_CODE >= NCCL_VERSION(2, 28, 0) but allreduceOp.cpp still references them unconditionally. Building with NCCL 2.27 causes compilation failure: error: 'NCCLWindowAllocator' has not been declared Bump NCCL minimum from 2.27.7 to 2.28.0 to satisfy the new version gate. Also bump cache version to 3 to invalidate stale wheels built against NCCL 2.27. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical build failure within the continuous integration environment for TensorRT-LLM by upgrading the minimum required NCCL library version. The change ensures compatibility with recent updates in TensorRT-LLM that introduced new symbols requiring a newer NCCL version, thereby stabilizing the CI pipeline and allowing successful builds. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThe CI installation script updates the NCCL runtime dependency from version 2.27.7 to 2.28.0 across two installation blocks and increments the cache version comment from 2 to 3. No logic or control flow changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Code Review
This pull request correctly bumps the minimum required NCCL version to 2.28.0 to fix a build failure with TensorRT-LLM. The changes are applied consistently for both cached and full builds. I've added one comment to suggest updating an outdated comment in the script to improve maintainability, aligning with the rule to ensure accurate external resource versions.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/ci_install_trtllm.sh (1)
159-163: 🧹 Nitpick | 🔵 TrivialUpdate stale section header comment.
The section header on line 159 still references "NCCL 2.27" but now installs 2.28+. Update for consistency.
📝 Suggested fix
-# ── NCCL 2.27 setup ────────────────────────────────────────────────────────── -# Use pip-installed NCCL 2.27+ which has both headers and libraries. +# ── NCCL 2.28 setup ────────────────────────────────────────────────────────── +# Use pip-installed NCCL 2.28+ which has both headers and libraries. # This matches the working installation guide approach. pip install --upgrade pip pip install --no-cache-dir "nvidia-nccl-cu13>=2.28.0"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ci_install_trtllm.sh` around lines 159 - 163, Update the stale section header that currently reads "── NCCL 2.27 setup ──────────────────────────────────────────────────────────" to reflect the actual package being installed (e.g., "── NCCL 2.28+ setup ──────────────────────────────────────────────────────────") so it matches the pip install line that installs "nvidia-nccl-cu13>=2.28.0"; ensure the header text near the pip install commands is consistent with the version constraint used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@scripts/ci_install_trtllm.sh`:
- Around line 159-163: Update the stale section header that currently reads "──
NCCL 2.27 setup ──────────────────────────────────────────────────────────" to
reflect the actual package being installed (e.g., "── NCCL 2.28+ setup
──────────────────────────────────────────────────────────") so it matches the
pip install line that installs "nvidia-nccl-cu13>=2.28.0"; ensure the header
text near the pip install commands is consistent with the version constraint
used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cdea1ede-ff9a-459f-96df-e717b3ec9f14
📒 Files selected for processing (1)
scripts/ci_install_trtllm.sh
…ct#777) Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
…ct#777) Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Summary
Root Cause
TRT-LLM PR #12015 (merged 2026-03-11, "Improve NCCL library load stability") wrapped
NCCLWindowAllocatorandcreateNCCLWindowTensorbehind#if NCCL_VERSION_CODE >= NCCL_VERSION(2, 28, 0)inncclUtils.h, butallreduceOp.cppstill references these symbols unconditionally. Building against NCCL 2.27 causes:What changed
nvidia-nccl-cu13>=2.27.7→nvidia-nccl-cu13>=2.28.0(both cached and full-build paths). Cache version bumped to 3 to invalidate stale wheels.Test plan
e2e-1gpu-chat (trtllm)job passesSummary by CodeRabbit