metal : simplify kernel arguments using a struct (#3229) #12194
Merged
Conversation
This file contains 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
github-actions
bot
added
ggml
ggerganov
reviewed
Mar 5, 2025
Comment on lines
289
to
290
| int32_t ofs0; | ||
| int32_t ofs1; |
There was a problem hiding this comment.
These have to be uint64_t since they are memory address strides. Also change the types in ggml_metal.m on lines 3281,3282 to uint64_t. Ideally there should be no division by 4 there - it assumes the data is float.
You can also just leave a comment TODO here and I'll adjust these later.
There was a problem hiding this comment.
Ok, I have changed the type.
added 4 commits
March 5, 2025 20:02
added 13 commits
March 6, 2025 15:33
|
@ggerganov I've completed the PR and would appreciate your review when you have time. I've already run all the tests locally and they're passing successfully. |
|
I've test this and can confirm that it fixes #12219. |
ggerganov
approved these changes
Mar 6, 2025
|
Just fix the trailing whitespaces and we can merge. |
Finished. |
|
Thank you. In case you are interested in contributing further, #12199 would be nice to fix. |
I'll see what I can do. |
mglambda
pushed a commit
to mglambda/llama.cpp
that referenced
this pull request
Mar 8, 2025
…l-org#12194) * metal : refactor im2col parameters into a struct * metal: Change im2col offset types from int32_t to uint64_t to support larger memory offsets * metal : refactor sum_rows parameters into a struct * metal : refactor soft_max parameters into a struct * metal : refactor diag_mask_inf parameters into a struct * metal : refactor ssm_conv parameters into a struct * metal : refactor ssm_scan parameters into a struct * metal : refactor get_rows parameters into a struct * metal : refactor group_norm parameters into a struct * metal : refactor conv_transpose_1d parameters into a struct * metal : refactor upscale parameters into a struct * metal : refactor pad parameters into a struct * metal : refactor pad_reflect_1d parameters into a struct * metal : refactor arange parameters into a struct * metal : refactor timestep_embedding parameters into a struct * metal : refactor argsort parameters into a struct * metal : refactor leaky_relu parameters into a struct * metal : refactor pool_2d parameters into a struct * metal : fix trailing whitespace --------- Co-authored-by: alexju <alexju@tencent.com>
arthw
pushed a commit
to arthw/llama.cpp
that referenced
this pull request
Mar 19, 2025
…l-org#12194) * metal : refactor im2col parameters into a struct * metal: Change im2col offset types from int32_t to uint64_t to support larger memory offsets * metal : refactor sum_rows parameters into a struct * metal : refactor soft_max parameters into a struct * metal : refactor diag_mask_inf parameters into a struct * metal : refactor ssm_conv parameters into a struct * metal : refactor ssm_scan parameters into a struct * metal : refactor get_rows parameters into a struct * metal : refactor group_norm parameters into a struct * metal : refactor conv_transpose_1d parameters into a struct * metal : refactor upscale parameters into a struct * metal : refactor pad parameters into a struct * metal : refactor pad_reflect_1d parameters into a struct * metal : refactor arange parameters into a struct * metal : refactor timestep_embedding parameters into a struct * metal : refactor argsort parameters into a struct * metal : refactor leaky_relu parameters into a struct * metal : refactor pool_2d parameters into a struct * metal : fix trailing whitespace --------- Co-authored-by: alexju <alexju@tencent.com>
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.
This PR implements the improvement suggested in issue #3229 by introducing a
ggml_metal_localsstruct to simplify Metal kernel arguments. This improvement reduces lengthy parameter lists, making the code more readable and maintainable.This draft PR currently only implements the optimization for the
im2colkernel function. I plan to continue working on optimizing the remaining kernel functions in subsequent commits.References
This PR follows the implementation pattern from PR #10238.