Skip to content

Conversation

@ye-NX
Copy link
Contributor

@ye-NX ye-NX commented Sep 17, 2025

Title:
sycl: add CONCAT support for GGML_TYPE_I16 and GGML_TYPE_I32

Body:

  • This PR generalizes the SYCL backend CONCAT operator to support GGML_TYPE_I16 and GGML_TYPE_I32.

  • Refactors float-only kernels into templated kernels (T) for dim0/1/2 and the non-contiguous fallback.

  • Replaces /4 with elem_size(dst->type) to ensure correct indexing for all types.

  • Adds a type switch in ggml_sycl_op_concat to invoke concat_impl_sycl for F32, I16, and I32.

  • No API changes; no behavior changes for other backends.

Testing

Built with -DGGML_SYCL=ON and verified correctness for F32, I16, I32 across dim 0/1/2/3 with both contiguous and non-contiguous inputs.

test-backend-ops passes for CONCAT.

@github-actions github-actions bot added testing Everything test related ggml changes relating to the ggml tensor library for machine learning SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language labels Sep 17, 2025
@ye-NX ye-NX force-pushed the ye/sycl-concat-additional-support branch from d889219 to 8d6da48 Compare September 21, 2025 07:24
@ye-NX
Copy link
Contributor Author

ye-NX commented Sep 21, 2025

You're right, the branch was based on an outdated master.
I’ve rebased onto upstream/master and force-pushed.

@NeoZhangJianyu
Copy link
Collaborator

@ye-NX
I think the PR should rebased to remove the unimpacted file, like ggufxxx.
The test-backend-op.cpp should be updated to restore the change of other legacy UT cases, except CONCAT OP.

@ye-NX ye-NX force-pushed the ye/sycl-concat-additional-support branch from 8d6da48 to aad3d99 Compare October 19, 2025 05:49
@ye-NX ye-NX requested a review from slaren as a code owner October 19, 2025 05:49
@ye-NX
Copy link
Contributor Author

ye-NX commented Oct 19, 2025

@NeoZhangJianyu
Thank you for reviewing! I’ve addressed the feedback:
– Rebased on the latest master and removed unrelated files (gguf*, b6524/b6527).
– Restored legacy tests in tests/test-backend-ops.cpp, keeping only the CONCAT changes.

Could you please take another look?

Copy link
Collaborator

@NeoZhangJianyu NeoZhangJianyu left a comment

Choose a reason for hiding this comment

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

The test-backend-ops.cpp has many change.
It will impact other backend.

Please only add new cases, instead of changing or deleting existed cases.

Thank you!

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 21, 2025
@github-actions github-actions bot added examples python python script changes devops improvements to build systems and github actions server labels Oct 21, 2025
@github-actions github-actions bot added Nvidia GPU Issues specific to Nvidia GPUs Vulkan Issues specific to the Vulkan backend OpenCL Issues specific to the OpenCL backend labels Oct 21, 2025
@NeoZhangJianyu
Copy link
Collaborator

@ye-NX
There are 76 files to be changed. :)
Maybe you need to rebase your branch.

@max-krasnyansky
Copy link
Collaborator

@ye-NX There are 76 files to be changed. :) Maybe you need to rebase your branch.

Yep. Looks like ggml-opencl changes got mixed in due to incorrect/incomplete rebase.

@allozaur allozaur removed their request for review October 22, 2025 16:22
Copy link
Collaborator

@NeoZhangJianyu NeoZhangJianyu left a comment

Choose a reason for hiding this comment

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

There are only 4 files needed.
But there are 6 empty files in this PR.
Could remove them?

image

@CISC CISC removed documentation Improvements or additions to documentation testing Everything test related Nvidia GPU Issues specific to Nvidia GPUs Vulkan Issues specific to the Vulkan backend examples python python script changes devops improvements to build systems and github actions server OpenCL Issues specific to the OpenCL backend labels Oct 30, 2025
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 30, 2025
@CISC CISC requested review from NeoZhangJianyu and ngxson October 30, 2025 10:30
@ye-NX
Copy link
Contributor Author

ye-NX commented Oct 30, 2025

@CISC , @NeoZhangJianyu
Can someone check if everything is okay now? Thanks!

Copy link
Collaborator

@NeoZhangJianyu NeoZhangJianyu left a comment

Choose a reason for hiding this comment

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

Good job!

@NeoZhangJianyu
Copy link
Collaborator

@ngxson
Please review again!
It's fixed for your comment.

@CISC
Copy link
Collaborator

CISC commented Nov 2, 2025

@ngxson Please unblock.

@ngxson ngxson merged commit 9d7c518 into ggml-org:master Nov 6, 2025
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation ggml changes relating to the ggml tensor library for machine learning SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants