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

Fix f16_sycl cpy call from Arc #5411

Merged
merged 5 commits into from
Feb 8, 2024

Conversation

abhilash1910
Copy link
Collaborator

@abhilash1910 abhilash1910 commented Feb 8, 2024

With respect to changes in #5289 , Arc has an issue for FP16 build when trying to query fp16_cpy on sycl . This is caused due to function arg mismatch , as it was calling an older version of the ggml_cpy_f32_f16_sycl method.

Error Log:

    |                                      ^
llama.cpp/ggml-sycl.cpp:12169:13: error: no matching function for call to 'ggml_cpy_f32_f16_sycl'
12169 |             ggml_cpy_f32_f16_sycl((const char *)src1_ddf_i, (char *)src1_dfloat,
       |             ^~~~~~~~~~~~~~~~~~~~~
llama.cpp/ggml-sycl.cpp:10638:13: note: candidate function not viable: requires 18 arguments, but 14 were provided
10638 | static void ggml_cpy_f32_f16_sycl(const char *cx, char *cdst, const int ne,
       |             ^                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10639 |                                   const int ne00, const int ne01,
       |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10640 |                                   const int ne02, const int nb00,
       |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10641 |                                   const int nb01, const int nb02,
       |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10642 |                                   const int nb03, const int ne10,
       |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10643 |                                   const int ne11, const int ne12,
       |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10644 |                                   const int nb10, const int nb11,
       |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10645 |                                   const int nb12, const int nb13,
       |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10646 |                                   dpct::queue_ptr stream) {
       |                                   ~~~~~~~~~~~~~~~~~~~~~~
103 warnings and 1 error generated.
gmake[2]: *** [CMakeFiles/ggml.dir/build.make:132: CMakeFiles/ggml.dir/ggml-sycl.cpp.o] Error 1
gmake[2]: Leaving directory 'llama.cpp/build'
gmake[1]: *** [CMakeFiles/Makefile2:738: CMakeFiles/ggml.dir/all] Error 2
gmake[1]: Leaving directory 'llama.cpp/build'
gmake: *** [Makefile:146: all] Error 2

@airMeng @AidanBeltonS @NeoZhangJianyu @ggerganov please review . Thanks

Copy link
Collaborator

@AidanBeltonS AidanBeltonS left a comment

Choose a reason for hiding this comment

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

LGTM, outside of my one comment. Thanks for catching this

ggml-sycl.cpp Outdated Show resolved Hide resolved
@NeoZhangJianyu
Copy link
Collaborator

I suggest adding CI case with build for FP16:
update .github\workflows\build.xml to add build CI for FP16.

So that such break building for FP16 will be detected.

Thank you!

ggml-sycl.cpp Outdated Show resolved Hide resolved
@abhilash1910 abhilash1910 merged commit 6e99f2a into ggerganov:master Feb 8, 2024
54 checks passed
jordankanter pushed a commit to jordankanter/llama.cpp that referenced this pull request Mar 13, 2024
* fix f16_sycl cpy call

* rm old logic

* add fp16 build CI

* use macro

* format fix
hodlen pushed a commit to hodlen/llama.cpp that referenced this pull request Apr 1, 2024
* fix f16_sycl cpy call

* rm old logic

* add fp16 build CI

* use macro

* format fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants