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

Add OpenCL add kernel #5151

Merged
merged 2 commits into from
Jan 26, 2024
Merged

Add OpenCL add kernel #5151

merged 2 commits into from
Jan 26, 2024

Conversation

0cc4m
Copy link
Collaborator

@0cc4m 0cc4m commented Jan 26, 2024

Fixes #5116

I took the simple route and added the GGML_OP_ADD op (by just copying and adapting the mul code). That should keep the OpenCL backend afloat until it gets superseded.

@0cc4m 0cc4m requested a review from slaren January 26, 2024 19:52
@slaren
Copy link
Collaborator

slaren commented Jan 26, 2024

I get this error when building with MSVC:

  ggml-opencl.cpp
C:\CODE\llama.cpp\ggml-opencl.cpp(727,1): error C2026: string too big, trailing characters truncated [C:\CODE\llama.cpp\build\ggml.vcxproj]

@0cc4m
Copy link
Collaborator Author

0cc4m commented Jan 26, 2024

I get this error when building with MSVC:

  ggml-opencl.cpp
C:\CODE\llama.cpp\ggml-opencl.cpp(727,1): error C2026: string too big, trailing characters truncated [C:\CODE\llama.cpp\build\ggml.vcxproj]

MSVC has a limitation on string length? Ugh. That would mean it was right up at the limit before I added the add kernel. I guess I can split it into a second string.

@slaren
Copy link
Collaborator

slaren commented Jan 26, 2024

Yes, it caused issues before. I think that's the reason why several kernels are in different strings already.

…h limit, disable float16 support due to bad results
@0cc4m
Copy link
Collaborator Author

0cc4m commented Jan 26, 2024

Yes, it caused issues before. I think that's the reason why several kernels are in different strings already.

No, all of the kernels in separate strings are there for templating reasons. It was pure luck that the huge string stayed in the limit so far. Can you try again?

Copy link
Collaborator

@slaren slaren left a comment

Choose a reason for hiding this comment

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

Works with phi2 for me.

@slaren
Copy link
Collaborator

slaren commented Jan 26, 2024

I could find a reference to the string length issue in #3973, but maybe it never got actually fixed.

@0cc4m 0cc4m merged commit a1d6df1 into master Jan 26, 2024
48 checks passed
@0cc4m 0cc4m deleted the 0cc4m/fix-opencl-bias-tensors branch January 26, 2024 22:07
@guilt
Copy link
Contributor

guilt commented Jan 29, 2024

@ggerganov OpenCL is the one compute acceleration thing that can run on the oldest computers and phones. At some point, please make a great release which still has good OpenCL support before abandoning it. Of course, it is quite possible to build exotic backends with Cg or GLSL, and keeping OpenCL is a good idea for everywhere there isn't Vulkan support.

GGML can 100% run on old GPUs through OpenCL 1.1; All it requires is a polyfill for one function last I checked it. What needs to be done to keep the OpenCL backend healthy? Please suggest.

@ggerganov
Copy link
Owner

I wrote about the OpenCL backend here: #5138

It can be updated similar to the rest of the backends given community enough support. There is no plan to completely abandon it - we just have to accept that at times it can be in a broken state

jordankanter pushed a commit to jordankanter/llama.cpp that referenced this pull request Feb 3, 2024
* Add OpenCL add kernel

* Put add kernel into different string to stay within MSVC string length limit, disable float16 support due to bad results
hodlen pushed a commit to hodlen/llama.cpp that referenced this pull request Apr 1, 2024
* Add OpenCL add kernel

* Put add kernel into different string to stay within MSVC string length limit, disable float16 support due to bad results
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.

CLBlast segfaulting with Phi q3_k_m and GPU offload (broken after backend integration)
4 participants