Repository navigation
Make _triton_permute_bwd handle non-contiguous grads and bad indices - #4989
Open
indianspeedster wants to merge 1 commit into
Open
indianspeedster wants to merge 1 commit into
indianspeedster wants to merge 1 commit into
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/4989
Note: Links to docs will display an error until the docs builds have been completed. This comment was automatically generated by Dr. CI and updates every 15 minutes. |
- The kernel addresses rows as `row * cols`, so a non-contiguous grad_output or permuted_indices gave wrong results with no error. Make both contiguous inside the op (a no-op when they already are). This covers the permute_mxfp8_fwd_hp_bwd backward, which passes grad_output through as-is. - The scatter only skipped the -1 padding value, so an index >= rows or any other negative index wrote outside output_buffer. Skip any index outside [0, original_rows). generate_permute_indices never emits one; this only matters if a caller passes inconsistent counts. Tests: test_triton_permute_bwd_non_contiguous_inputs covers a transposed gradient and a strided index tensor. test_triton_permute_bwd_kernel_out_of_range_indices launches the kernel into the middle of a larger guard buffer and checks the guard rows are untouched for indices past the end and below zero.
indianspeedster
force-pushed
the
fix/permute-bwd-bounds
branch
from
October 8, 2026 19:52
e47b5dd to
f9ff209
Compare
indianspeedster
marked this pull request as ready for review
October 8, 2026 20:03
indianspeedster
requested review from
andrewor14,
jerryzh168 and
vkuzo
as code owners
October 8, 2026 20:03
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two robustness fixes to
_triton_permute_bwd:row * cols, so anon-contiguous
grad_outputorpermuted_indicesproduced wrong results withno error. The op now makes both contiguous itself (a no-op when they already
are). This covers the
permute_mxfp8_fwd_hp_bwdbackward, which passesgrad_outputthrough as-is.-1padding value, soan index
>= rows, or any other negative index, wrote outsideoutput_buffer.It now skips any index outside
[0, original_rows).generate_permute_indicesnever emits one, so this only matters if a callerpasses inconsistent counts.
Split out of #4623 as requested in review.
Test plan
New tests in
test/prototype/moe_training/ep/test_kernels.py:test_triton_permute_bwd_non_contiguous_inputs: a transposed gradient and astrided index tensor must give the same result as their contiguous copies.
test_triton_permute_bwd_kernel_out_of_range_indices: launches the kernel intorows
[guard, guard + rows)of a larger buffer filled with a sentinel, withindices
rows,rows + 3,-2and-7. The guard rows must stayuntouched and the valid rows must be scattered correctly.
test_permute.py+ep/test_kernels.pywith this changemain's kernel< original_rows)>= 0).contiguous()On MI355X the stray writes for negative indices don't land in the guard rows even
without the lower bound, so that case is only observable on B200.
Test command on both machines:
pytest test/prototype/moe_training/ep/test_permute.py test/prototype/moe_training/ep/test_kernels.py. MI355X: torch 2.15.0.dev+rocm7.14, triton 3.8.0; becausemaingates these files to SM100, the MI355X runs override that gate locally. B200: torch 2.14.1+cu130, triton 3.8.0.No upstream CI runner has SM100, and these test files are gated to SM100 on
main, so today these tests run in no CI job. After #4987 (which enables them on gfx950) they run in the ROCm regression job (regression_test_rocm.yml, MI350 runner, triggered byciflow/rocmor a push tomain).This PR and #4988 both append tests to the end of
ep/test_kernels.py, sowhichever lands second needs a trivial rebase.
Authored with assistance from an AI coding agent.