Skip to content

[None][fix] Remove unused WarpPolicy in MOE OneSided A2A to reduce ma… - #14539

Closed
djns99 wants to merge 1 commit into
NVIDIA:mainfrom
djns99:djns99/disable_a2a_warp_policy
Closed

[None][fix] Remove unused WarpPolicy in MOE OneSided A2A to reduce ma…#14539
djns99 wants to merge 1 commit into
NVIDIA:mainfrom
djns99:djns99/disable_a2a_warp_policy

Conversation

@djns99

@djns99 djns99 commented May 26, 2026

Copy link
Copy Markdown
Collaborator

…intenance and prevent silent regressions

Summary by CodeRabbit

Release Notes

  • Changes
    • MOE A2A communication now permanently uses one-block-per-token scheduling mode and cannot be disabled via environment variable.
    • Setting TLLM_MOE_A2A_ONE_BLOCK_PER_TOKEN will emit a warning that the feature is always enabled.
    • Deprecated getEnvMoeA2AOneBlockPerToken() function.

Review Change Stack

Description

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)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • 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.

…intenance and prevent silent regressions

Signed-off-by: djns99 <40156487+djns99@users.noreply.github.com>
@djns99
djns99 requested a review from bobboli May 26, 2026 02:29
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR enforces one-block-per-token mode as the exclusive scheduling strategy for MOE A2A operations. The environment utility function is hardcoded to return true with deprecation guidance, and kernel code removes WarpPolicy specialization entirely, keeping only BlockPolicy and simplifying shared-memory layout and launch logic accordingly.

Changes

MOE A2A One-Block-Per-Token Mode Enforcement

Layer / File(s) Summary
Environment utility enforcement
cpp/tensorrt_llm/common/envUtils.cpp, cpp/tensorrt_llm/common/envUtils.h
getEnvMoeA2AOneBlockPerToken() is rewritten to unconditionally return true and log a warning if the environment variable is set; copyright years are updated to 2022–2026, and function documentation is updated to mark it deprecated and state it always returns true.
Kernel policy simplification and BlockPolicy enforcement
cpp/tensorrt_llm/kernels/communicationKernels/moeAlltoAllKernels.cu
WarpPolicy struct definition and warp-based launch sizing helpers are removed; SWITCH_POLICY macro now rejects WarpPolicy with a check; moeA2ADispatchKernel shared-memory layout is simplified to a single fixed layout; moe_a2a_dispatch_launch and moeA2APrepareCombineKernel launch functions throw when one_block_per_token is false and always select BlockPolicy.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete with empty sections for Description and Test Coverage, though the checklist is marked complete. Fill in the Description section explaining the issue and solution, and add Test Coverage section listing relevant tests for the changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: removing unused WarpPolicy in MOE A2A to improve maintenance.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@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.

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/communicationKernels/moeAlltoAllKernels.cu (1)

1-2: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update copyright year for modified file.

The file is being modified but the copyright year on line 2 still shows only 2025. Per coding guidelines, the year should be updated on modified files.

Suggested fix
 /*
- * Copyright (c) 2025, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2025-2026, NVIDIA CORPORATION.  All rights reserved.
  *

As per coding guidelines: **/*.{cpp,cc,h,hpp,py,cu,cuh}: Include NVIDIA copyright header on ALL new files; update year on modified files.

🤖 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 `@cpp/tensorrt_llm/kernels/communicationKernels/moeAlltoAllKernels.cu` around
lines 1 - 2, Update the copyright header at the top-of-file comment to include
the current year range (e.g., change "2025" to "2025-2026") so the modified
file's header is current; edit the top comment block in moeAlltoAllKernels.cu
(the file-level /* ... */ header) to reflect the updated year range and ensure
formatting matches existing NVIDIA copyright header style.
🤖 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.

Outside diff comments:
In `@cpp/tensorrt_llm/kernels/communicationKernels/moeAlltoAllKernels.cu`:
- Around line 1-2: Update the copyright header at the top-of-file comment to
include the current year range (e.g., change "2025" to "2025-2026") so the
modified file's header is current; edit the top comment block in
moeAlltoAllKernels.cu (the file-level /* ... */ header) to reflect the updated
year range and ensure formatting matches existing NVIDIA copyright header style.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5749f47e-8912-4ca7-b30f-152cb4410723

📥 Commits

Reviewing files that changed from the base of the PR and between 7142d07 and 08d3bfd.

📒 Files selected for processing (3)
  • cpp/tensorrt_llm/common/envUtils.cpp
  • cpp/tensorrt_llm/common/envUtils.h
  • cpp/tensorrt_llm/kernels/communicationKernels/moeAlltoAllKernels.cu

@djns99

djns99 commented May 26, 2026

Copy link
Copy Markdown
Collaborator Author

@bobboli will close this in a future PR

@djns99 djns99 closed this May 26, 2026
@bobboli

bobboli commented May 26, 2026

Copy link
Copy Markdown
Collaborator

Superseded by #14550

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.

2 participants