Skip to content

Conversation

@AidanBeltonS
Copy link
Contributor

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
Contributor

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
Contributor

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
Contributor 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
Contributor

@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

@AidanBeltonS AidanBeltonS force-pushed the reduce_warning_messages branch from 9865df7 to 4d89e62 Compare July 3, 2024 08:50
@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
Contributor

@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!

@AidanBeltonS AidanBeltonS force-pushed the reduce_warning_messages branch from 70bd412 to cf94e5d Compare July 10, 2024 09:28
@OuadiElfarouki OuadiElfarouki merged commit f4444d9 into ggml-org:master Jul 10, 2024
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