ROCm / ROCm/iris

Fix pre-existing kernel issues found during CCL split

Open
#524 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

gluon iris
Dominant language
Python
Stars
202
Forks
47
Avg merge
6d 11h
Merged PRs (30d)
4

Description

Context

During the CCL split refactor (#523), Copilot code review flagged several pre-existing issues in kernel code. These exist on main today and were not introduced by the split. Filing here to track as follow-up work.

Issues

1. Gluon all-to-all: % M/% N modulo wrapping on boundary tiles

File: iris/ccl/gluon/all_to_all.py (lines 94, 108, 113)

rm = (...) % M and rn = (...) % N causes boundary tiles to wrap back to the start of the tensor. After wrapping, col_mask = rn < N is trivially true, so out-of-bounds elements are never masked. Could produce incorrect results when M or N is not an exact multiple of block size.

Fix: Remove % M/% N, use proper row/col masks like the Triton backend. Needs GPU testing to verify the modulo isn't intentional for gluon's masking model.

2. Gluon chiplet_transform: > vs >= off-by-one

File: iris/ccl/gluon/all_to_all.py (line 25)

if pid > (num_workgroups // (num_xcds * chunk_size)) * (num_xcds * chunk_size):

Should be >= — the first tail PID should not be transformed. Same pattern exists in iris/ccl/utils.py.

3. Triton all-gather: unused NUM_XCDS/CHUNK_SIZE parameters

File: iris/ccl/triton/all_gather.py (lines 35, 61)

Both persistent_all_gather and persistent_all_gather_partitioned accept NUM_XCDS/CHUNK_SIZE but never call chiplet_transform_chunked. Other backends (all_reduce, reduce_scatter, all_to_all) all apply the transform. Either apply it or remove the parameters.

4. Ring all-reduce: SLICE_SIZE_N passed but unused

File: iris/ccl/triton/all_reduce.py (lines 447, 454, 573)

SLICE_SIZE_N is passed to the ring kernel and launch() enforces slice-related constraints, but the kernel body ignores it and always uses BLOCK_SIZE_N. Either implement column slicing or remove the parameter + validation.

Testing

All fixes need multi-GPU testing on MI300X+ hardware:

torchrun --nproc_per_node=8 tests/run_tests_distributed.py tests/ccl/ -v

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the four named files and lines: iris/ccl/gluon/all_to_all.py, iris/ccl/utils.py, iris/ccl/triton/all_gather.py, and iris/ccl/triton/all_reduce.py; compare the Gluon masking with the Triton backend and trace the affected kernel parameters. Run torchrun --nproc_per_node=8 tests/run_tests_distributed.py tests/ccl/ -v on MI300X+ hardware; done means the issues are resolved without regressions in the multi-GPU tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
distributed-systems, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.