Repository navigation
Conversation
concatenate_gpu built each destination view with copy_shared_buffer and a data_size equal to the view's element count. The copy dispatch picks 32-bit indexing when data_size fits in INT32_MAX, so a destination whose rows start beyond 2^31 elements was written with wrapped offsets (garbage past the boundary). Build the view with slice() so data_size is the reachable span, exactly as the pad fix does. Regression test mirrors test_pad_large_offsets. (cherry picked from commit b1f132a)
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.
Proposed changes
concatenate_gpu(Metal and CUDA) builds each destination view withcopy_shared_bufferand adata_sizeequal to the view's element count. The copy dispatch selects 32-bit indexing whendata_sizefits inINT32_MAX, so a destination whose rows start beyond 2^31 elements is written with wrapped offsets and everything past the boundary is garbage, while the output as a whole can still be well under the 2^32 size limit.This builds the destination view with
slice()instead, so itsdata_sizeis the span it reaches, exactly as #4744 did forpad. Same class of issue as thesplitfix in #4540.Reproduction (before this change, on an M2 Max):
concatenate([q, k, v], axis=-1)with three[105000, 7168]bfloat16 inputs returns correct rows up to 99,864 and wrong rows after that (2^31 / 21504 = 99,864). We hit it in a video transformer's fused Q/K/V projection at long sequence lengths.The added test mirrors
test_pad_large_offsets: row 2^27 of auint8(2^27 + 1, 16)output starts at offset 2^31.Checklist