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

ggml.c CACHE_LINE_SIZE error: initializer element is not constant #11

Closed
biemster opened this issue Oct 1, 2022 · 4 comments
Closed
Labels
enhancement New feature or request

Comments

@biemster
Copy link

biemster commented Oct 1, 2022

I get this on Ubuntu 18.04 gcc 7.5.0 (time to update, yes), and I don't immediately see how to fix it since I don't know __cpp_lib_hardware_interference_size. Otherwise a simple replacement with a #define would suffice.

gcc -pthread -O3 -mavx -mavx2 -mfma -mf16c -c ggml.c
ggml.c:183:36: error: initializer element is not constant
 const size_t CACHE_LINE_SIZE_F32 = CACHE_LINE_SIZE/sizeof(float);
@biemster
Copy link
Author

biemster commented Oct 1, 2022

Also a few

main.cpp:1349:62: sorry, unimplemented: non-trivial designated initializers not supported
             struct ggml_cgraph gf = { .n_threads = n_threads };

so it is really time to update. You can close this if you choose not to support old compilers (although to my defense this is not THAT old..)

@biemster
Copy link
Author

biemster commented Oct 1, 2022

Hold on, I'm being to quick here. The initial issue can actually be solved by replacing the const size_t with a #define.

@ggerganov ggerganov added the enhancement New feature or request label Oct 1, 2022
@ggerganov
Copy link
Owner

ggerganov commented Oct 1, 2022

Regarding CACHE_LINE_SIZE - we need this to be at least as big as the L1 cache line size to avoid false sharing between threads during computation. I don't see why gcc 7.5.0 thinks this is not constant, but I'm okay to replace it with #define if that solves the issue.

Regarding the designated initializers in the .cpp - just replace it with:

struct ggml_cgraph gf; gf.n_threads = n_threads;

Feel free to submit a PR with these changes and I will be happy to apply them


Edit Oct 7:

The suggested fix above is wrong. It should be:

struct ggml_cgraph gf = {}; gf.n_threads = n_threads;

Already fixed on master

@biemster
Copy link
Author

biemster commented Oct 1, 2022

Thanks! That works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants