Skip to content
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.

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

SYCL: Refactor ggml_sycl_compute_forward #11121

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

qnixsynapse
Copy link
Contributor

I originally thought I wouldn't do this, but the original code is quite restrictive imo. It assumes that all operations need only src0, src1 and dst tensors, which isn't the case for every OP(also, why are we passing them separately?). This change will allow easy addition of tensor operations to the backend as the dst tensor itself contains everything needed to perform the operation. I haven't changed the kernels except for a few function signature changes, as that's outside the scope of this refactoring. Those can be refactored later if needed.

@github-actions github-actions bot added ggml changes relating to the ggml tensor library for machine learning SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language labels Jan 7, 2025
Copy link
Collaborator

@Alcpz Alcpz left a comment

Choose a reason for hiding this comment

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

Code looks simpler now. Thanks for the change.

Edit: Will wait for other to give a review.

@qnixsynapse
Copy link
Contributor Author

@Alcpz Thanks for your review.
Tagging @Rbiessy @NeoZhangJianyu @airMeng manually to get their thoughts on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

2 participants