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] Use multi_ptr to clean up deprecated warnings #8256

Merged
merged 5 commits into from
Jul 10, 2024

Conversation

AidanBeltonS
Copy link
Collaborator

This PR adds a get_pointer() helper function to use the more up-to-date get_multi_ptr.

This cleans up build warnings significantly.

@HanClinto
Copy link
Collaborator

We just fixed the problems that editorconfig is complaining about, so if you update with a rebase or merge with master, you should be able to get a clean bill-of-health on the CI.

@airMeng
Copy link
Collaborator

airMeng commented Jul 3, 2024

Can you remove the other get_pointer too?

$:~/llama.cpp/ggml$ grep -rI "get_pointer()"
src/ggml-sycl.cpp:                                                                             local_buf_acc.get_pointer());
src/ggml-sycl/dpct/helper.hpp:            : accessor(acc.get_pointer(), in_range) {}
src/ggml-sycl/dpct/helper.hpp:            : accessor(acc.get_pointer(), in_range) {}

@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 Jul 3, 2024
@AidanBeltonS
Copy link
Collaborator Author

Can you remove the other get_pointer too?

$:~/llama.cpp/ggml$ grep -rI "get_pointer()"
src/ggml-sycl.cpp:                                                                             local_buf_acc.get_pointer());
src/ggml-sycl/dpct/helper.hpp:            : accessor(acc.get_pointer(), in_range) {}
src/ggml-sycl/dpct/helper.hpp:            : accessor(acc.get_pointer(), in_range) {}

Thanks for pointing these out. I have addressed the src/ggml-sycl.cpp file.

I have not touched the dpct helpers, I am working on a separate PR to clean up some warnings in that. I will include the changes in the second PR, as I want to provide feedback to DPCT and provide an alternative that is independent of llama.cpp.

Copy link
Collaborator

@joeatodd joeatodd left a comment

Choose a reason for hiding this comment

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

LGTM - thanks @AidanBeltonS, it's a tidy solution

ggml/src/ggml-sycl/common.hpp Outdated Show resolved Hide resolved
@mofosyne mofosyne added the Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix label Jul 3, 2024
Copy link
Collaborator

@OuadiElfarouki OuadiElfarouki left a comment

Choose a reason for hiding this comment

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

Looks good!

@OuadiElfarouki OuadiElfarouki merged commit f4444d9 into ggerganov:master Jul 10, 2024
53 checks passed
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Jul 13, 2024
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 Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants