Skip to content

Add CONV_TRANSPOSE_2D for Metal #16542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add CONV_TRANSPOSE_2D for Metal #16542

wants to merge 8 commits into from
+176 −0

Conversation

iliailmer
Copy link

@iliailmer iliailmer commented Oct 12, 2025 •
edited
Loading

This PR adds Metal-based implementation of CONV_TRANSPOSE_2D operation (#14909)

TODO:

  • Tests

@iliailmer iliailmer marked this pull request as ready for review October 12, 2025 20:17
@iliailmer iliailmer requested a review from ggerganov as a code owner October 12, 2025 20:17
@github-actions github-actions bot added ggml changes relating to the ggml tensor library for machine learning Apple Metal https://en.wikipedia.org/wiki/Metal_(API) labels Oct 12, 2025
ggerganov
ggerganov reviewed Oct 13, 2025
View reviewed changes
Copy link
Member

@ggerganov ggerganov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the necessary requirements for the input tensors here:

llama.cpp/ggml/src/ggml-metal/ggml-metal-device.m

Lines 649 to 651 in a190a9d

case GGML_OP_CONV_TRANSPOSE_1D:
case GGML_OP_CONV_TRANSPOSE_2D:
return true;

For example, the implementation assumes that src0 and src1 are contiguous.

@iliailmer iliailmer changed the title Add metal conv transpose 2d Add CONV_TRANSPOSE_2D for Metal Oct 13, 2025
@iliailmer iliailmer changed the title Add CONV_TRANSPOSE_2D for Metal Add CONV_TRANSPOSE_2D for Metal Oct 13, 2025
@iliailmer
Copy link
Author

Added the checks for type, and is_contiguous checks as well.

@iliailmer iliailmer requested a review from ggerganov October 14, 2025 00:23
ggerganov
ggerganov reviewed Oct 14, 2025
View reviewed changes
Copy link
Member

@ggerganov ggerganov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be more efficient to have KH x KW threads in each threadgroup, instead of just 1.

Would you like to try that in this PR or in a follow-up PR?

ggml/src/ggml-metal/ggml-metal.metal

if (in_y >= args.IH) continue;

for (int64_t kw = 0; kw<args.KW; kw++) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (int64_t kw = 0; kw<args.KW; kw++) {
for (int64_t kw = 0; kw < args.KW; kw++) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ggerganov ggerganov ggerganov left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Apple Metal https://en.wikipedia.org/wiki/Metal_(API) ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@iliailmer @ggerganov