Repository navigation
[Fix] Use the retained dimensions when slicing a register - #172
Merged
Merged
Conversation
SliceRegisterInst and SliceAssignInst keep the dimensions in inst.dims, but the emitters indexed the indices of the sliced tensor by source dimension. Slice reads also took those indices from the source layout, so r[:, 1] on local(4, 4) read regs[1] for every element, and r[1, :] = v raised IndexError. Read the indices from the sliced tensor's layout and map them by their position in dims. Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
…dims When a slice assign drops a destination dim that is distributed across threads (e.g. r[1, :] = v with r in spatial(4, 1).local(1, 4)), every thread wrote its local copy and overwrote the other rows. Only threads that own the selected slice now write. Tests: cover multi-thread assigns and a 3-D slice with non-contiguous retained dims, assert regex matches before use, and drop the dead CPU device fallback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
|
/ok to test dd43c37 |
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.
What
Slicing a register along anything but its trailing dimensions is broken:
r[:, 1]on alocal(4, 4)register emitsslice_regs[i] = regs[1]for every element, instead ofregs[i * 4 + 1]. No error is raised, and the kernel gets four copies of one value.r[1, :] = vraisesIndexErrorin the emitter.Why
inst.dimslists the source dimensions the slice keeps, so the sliced tensor haslen(dims)dimensions. The emitters got this wrong in two ways:SliceRegisterInstEmittercomputed the sliced tensor's indices from the source layout (src_layout.get_global(...)) with the sliced tensor's local index.dst_indices[dim]), not by position indims.The two mistakes cancel when the kept dimensions are the trailing ones, for example
r[1, :]or thephiloxoutput slices. So those cases worked and the bug stayed hidden.How
Take the indices from the sliced tensor's layout, and map them with
enumerate(dims). The same mapping is applied toSliceAssignInstEmitterand to the const-register path. Tracked const registers are 1-D today, sodimand its position are the same there and that path behaves as before.Testing
Host tests ran on the Grace CPU of a GH200: GCC 13.3, CUDA 13.2, PyTorch 2.14.1+cu130, tilus
mainat f23018a.tests/instructions/test_slice_register.pyis new. The codegen tests evaluate the register index in the generatedsource.cufor each loop iteration:maintest_read_slice_codegen[1]:r[:, 1]reads 1, 5, 9, 13test_assign_slice_codegen[0]:r[1, :] = vwrites 4, 5, 6, 7IndexErrortest_read_slice_codegen[0],test_assign_slice_codegen[1](controls)test_read_slice[1],test_assign_slice[0]test_read_slice[0],test_assign_slice[1],test_read_column_across_threads(controls)GPU tests ran on GH200 (
sm_90, CUDA 13.2). Not run on B300.To check that working slices do not change, I compared the generated
source.cuofmainand this PR for 38 programs:tests/instructionsscripts,hopper_matmulv0 to v5,blackwell_matmulv0 to v3 and the threephiloxkernels with two layouts each. 21 of them contain register slices. All 38 are byte-identical.The rest of the CPU-runnable suite (
tests/lang,tests/ir,tests/drivers,tests/transformsand the top-level tests) gives the same result onmainand here: 192 passed, and the same 5 failed because they need a GPU.ruff checkandruff format --check(0.11.0 frompyproject.toml, 0.12.3 from pre-commit),mypy(1.15.0) and the copyright check pass on the changed files.Not in this PR
These behave the same on
mainand with this PR:r[1, :] += vfails in the transpiler.r[1, :] = 5fails layout inference.r[1, :]on a register whose dimension 0 is spatial (spatial(4, 1).local(1, 4)) emitsslice_regs[i] = regs[i], so each thread reads its own row and not row 1. Fixing that needs communication between threads, or an error.