Skip to content

Use fused pad+MCT when available#142

Open
alextmagro wants to merge 2 commits into
rocm_devfrom
fused_padding_mct
Open

Use fused pad+MCT when available#142
alextmagro wants to merge 2 commits into
rocm_devfrom
fused_padding_mct

Conversation

@alextmagro

Copy link
Copy Markdown

fuses the padding kernels and cast transpose kernels that are used for FP8 MoE operations. Fuses both in forward and backward pass.

@sudhu2k sudhu2k left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Otherwise looks good to me.

if HAVE_TE and is_rocm:
HAVE_FUSED_PADDING_MCT = hasattr(Fp8Padding, 'compute_padded_splits')
if not HAVE_FUSED_PADDING_MCT:
logging.getLogger(__name__).info(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use the logger object from line 72 to do
Use logger.warning(
"message"
) instead

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds an ROCm/TransformerEngine-gated fast path for FP8/FP4 MoE that fuses padding with TE’s cast+transpose (MCT) in grouped GEMMs, aiming to avoid BF16 intermediate padding and optionally unpad FC2 outputs.

Changes:

  • Detect TE support for fused padding (via Fp8Padding.compute_padded_splits) and use it to compute padded m_splits without materializing a BF16-padded activation.
  • Plumb actual_m_splits (and unpad_output for FC2) through TEGroupedMLP into the TE GroupedLinear wrapper when supported by the installed TE version.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
megatron/core/transformer/moe/experts.py Adds ROCm fused padding path and passes actual_m_splits / unpad_output into grouped linears.
megatron/core/extensions/transformer_engine.py Extends TE GroupedLinear wrapper signature and conditionally forwards fused-padding kwargs to TE.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +662 to +665
if HAVE_FUSED_PADDING_MCT:
# ROCm: fuse padding into cast_transpose — compute padded
# splits without allocating a BF16 intermediate.
tokens_per_expert = self.quantization_padding.compute_padded_splits(tokens_per_expert)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We don't have a fp8 triton groupedgemm kernel for now and we would fall back to hipblasLT Multistream GEMM. But I guess it would be future proof to do this and the better way would be to pad on the gpu tensors directly instead of creating a new tensor from a list.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think maybe this should be postponed until we have intentions to do a grouped gemm in triton.

Comment thread megatron/core/transformer/moe/experts.py Outdated
Comment thread megatron/core/extensions/transformer_engine.py Outdated
@alextmagro
alextmagro requested a review from sudhu2k June 26, 2026 18:01
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.

3 participants