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

Use aligned_alloc() if aligned allocations are required #880

Closed
4 tasks done
Rhialto opened this issue Apr 10, 2023 · 4 comments · Fixed by #884
Closed
4 tasks done

Use aligned_alloc() if aligned allocations are required #880

Rhialto opened this issue Apr 10, 2023 · 4 comments · Fixed by #884

Comments

@Rhialto
Copy link

Rhialto commented Apr 10, 2023

Prerequisites

Please answer the following questions for yourself before submitting an issue.

  • I am running the latest code. Development is very rapid so there are no tagged versions as of now.
  • I carefully followed the README.md.
  • I searched using keywords relevant to my issue to make sure that I am creating a new issue that is not already open (or closed).
  • I reviewed the Discussions, and have a new bug or useful enhancement to share.

Expected Behavior

quantize should run and not abort with an assertion failure

Current Behavior

GGML_ASSERT: ggml.c:2990: ((uintptr_t) (ctx->mem_buffer))%GGML_MEM_ALIGN == 0
Abort trap (core dumped)

Environment and Context

  • Operating System

NetBSD 9.3

Failure Information (for bugs)

The memory allocated by malloc is alingned to 8, not 16 (GGML_MEM_ALIGN)

Fix:

diff --git a/ggml.c b/ggml.c
index 326b8e8..efa3f15 100644
--- a/ggml.c
+++ b/ggml.c
@@ -2974,7 +2974,7 @@ struct ggml_context * ggml_init(struct ggml_init_params params) {
 
     *ctx = (struct ggml_context) {
         /*.mem_size           =*/ params.mem_size,
-        /*.mem_buffer         =*/ params.mem_buffer ? params.mem_buffer : malloc(params.mem_size),
+        /*.mem_buffer         =*/ params.mem_buffer ? params.mem_buffer : aligned_alloc(GGML_MEM_ALIGN , params.mem_size),
         /*.mem_buffer_owned   =*/ params.mem_buffer ? false : true,
         /*.no_alloc           =*/ params.no_alloc,
         /*.n_objects          =*/ 0,
@prusnak
Copy link
Collaborator

prusnak commented Apr 10, 2023

The proposed fix does not work on Windows (MSVC), since it does not implement aligned_alloc.

@j-f1
Copy link
Collaborator

j-f1 commented Apr 10, 2023

Specifically, you’d need to use _aligned_malloc on Windows, and then make sure to use _aligned_free to free the buffer, also making sure to use _aligned_realloc if realloc is ever used.

@prusnak
Copy link
Collaborator

prusnak commented Apr 10, 2023

Specifically, you’d need to use ...

Thanks! Created #884

@Rhialto can you please check this fixes the issue for you?

@Rhialto
Copy link
Author

Rhialto commented Apr 11, 2023

@prusnak Yes thanks! That works for me!

jeroen-mostert pushed a commit to jeroen-mostert/llama.cpp that referenced this issue Aug 30, 2024
Fixes compile issues on Windows with w64devkit
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 a pull request may close this issue.

3 participants