Skip to content

opencl : fix memory allocation size #12649

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

Merged
merged 1 commit into from
Apr 1, 2025

Conversation

sparkleholic
Copy link
Contributor

issue:
CodeLinaro#17 (comment)

This patch fixes the memory allocation size
not exceeding the maximum size of the OpenCL device.

issue:
CodeLinaro#17 (comment)

This patch fixes the memory allocation size
not exceeding the maximum size of the OpenCL device.
@github-actions github-actions bot added the ggml changes relating to the ggml tensor library for machine learning label Mar 30, 2025
@sparkleholic
Copy link
Contributor Author

sparkleholic commented Mar 30, 2025

@lhez, @bandoti <

Please take a look at this patch. I think it could help address the issue where the existing code exceeds MAX_MEM_ALLOC_SIZE during memory allocation. It might serve as a temporary solution until we come up with a more refined fix. What do you think?

@sparkleholic
Copy link
Contributor Author

@lhez is not responding to my request. @max-krasnyansky, Could you review this PR?
Current implementation might cause runtime error while calling clCreateBuffer with the size exceeding the OpenCL device's maximum memory allocation size.
This patch prevents this bug.

@bandoti bandoti requested a review from max-krasnyansky April 1, 2025 01:52
Copy link
Collaborator

@max-krasnyansky max-krasnyansky left a comment

Choose a reason for hiding this comment

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

Yep. Looks good to me (i.e. better than the original version).

@lhez we should add some comments about how those A_q_*_bytes magic numbers were computed.

@max-krasnyansky max-krasnyansky merged commit f423981 into ggml-org:master Apr 1, 2025
47 of 48 checks passed
@lhez
Copy link
Contributor

lhez commented Apr 1, 2025

Yeah, we need to add some comments. Did some calculation last night and I think I recalled the calculation. I think we should make the calculation more formal and print out limitations when allocation is limited.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants