Skip to content

b1705 introduces build error on linux-aarch64 #4654

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

Closed
jhheider opened this issue Dec 27, 2023 · 9 comments · Fixed by #4715
Closed

b1705 introduces build error on linux-aarch64 #4654

jhheider opened this issue Dec 27, 2023 · 9 comments · Fixed by #4715
Labels
bug-unconfirmed help wanted Extra attention is needed

Comments

@jhheider
Copy link

Starting with b1705, I get the following when building for linux-aarch64:

clang++ -I. -Icommon -D_XOPEN_SOURCE=600 -D_GNU_SOURCE -DNDEBUG  -std=c++11 -fPIC -O3 -Wall -Wextra -Wpedantic -Wcast-qual -Wno-unused-function -Wmissing-declarations -Wmissing-noreturn -pthread -mcpu=native   -Wunreachable-code-break -Wunreachable-code-return -Wmissing-prototypes -Wextra-semi -c common/train.cpp -o train.o
ggml-quants.c:413:25: error: redefinition of 'vdotq_s32'
inline static int32x4_t vdotq_s32(int32x4_t acc, int8x16_t a, int8x16_t b) {
                        ^
/opt/llvm.org/v16.0.6/lib/clang/16/include/arm_neon.h:33859:51: note: previous definition is here
__ai __attribute__((target("dotprod"))) int32x4_t vdotq_s32(int32x4_t __p0, int8x16_t __p1, int8x16_t __p2) {
                                                  ^
1 error generated.

it seems to be directly related to #4630. the various #define tricks i'm used to don't seem to be able to resolve it. any ideas?

full build log: https://github.com/pkgxdev/pantry/actions/runs/7340844999/job/19987542307

@after-ephemera
Copy link

after-ephemera commented Dec 28, 2023

Also saw this on an iOS build last night, looking for a solution.

Edit: fwiw reverting to f56d607 worked to get it building again.

@1-ashraful-islam
Copy link
Contributor

1-ashraful-islam commented Dec 30, 2023

I am getting the same error when trying to compile the project in XCode.
When using simulated iOS device, I don't get the error. But when I select physical device (iPhone 12 pro max) I see this redefinition of vdotq_s32 error. It is related to #4630

@ggerganov
Copy link
Member

Not sure how to fix this gracefully - open to suggestions

@ggerganov ggerganov added the help wanted Extra attention is needed label Dec 30, 2023
@slaren
Copy link
Member

slaren commented Dec 30, 2023

Is it possible to use a different name for vdotq_s32 in the operations, and #define it to either the original instruction or the fallback depending on the flags?

@svilupp
Copy link

svilupp commented Dec 30, 2023

+1

I see this when producing binaries for Apple Silicon on MacOS ("aarch64-apple-darwin").
Interestingly, I see this error only in the buildkite (link). I can build it locally just fine.

@ggerganov
Copy link
Member

@slaren This is what we did for some of the 64-bit ARM ops to make the build compatible with 32-bit ARM:

https://github.com/ggerganov/llama.cpp/blob/39d8bc71edcb8b6f99d46fa4216af7a15232e218/ggml-quants.c#L394-L409

However, for vdotq_s32 I don't know how to check at compile time if it is present or not. I thought that __ARM_FEATURE_DOTPROD is the correct define to check and it works on the various Android and Raspberry devices that I've tested, but surprisingly it fails with Xcode builds.

For now I'm planning to change the check from:

https://github.com/ggerganov/llama.cpp/blob/39d8bc71edcb8b6f99d46fa4216af7a15232e218/ggml-quants.c#L411-L420

to:

#if defined(GGML_ARM_DOTPROD)
...

This way, on older Arm devices that do not have vdotq_s32 available we will get a compile error and the solution would be to build with -DGGML_ARM_DOTPROD in such cases.

@slaren
Copy link
Member

slaren commented Dec 30, 2023

Maybe I am completely wrong about this, but I am assuming that the issue is that the dotprod intrinsic is defined even if the system doesn't support it, and that causes a name clash with the fallback function. So if that is the case, using a different name should fix it. From what I can tell, before #4630 we also relied on __ARM_FEATURE_DOTPROD, so at worst this would restore the previous behavior.

@ggerganov
Copy link
Member

You are correct - I've pushed a fix as suggested in #4715

@jhheider
Copy link
Author

jhheider commented Jan 3, 2024

can confirm that the build error is no longer present as of b1732.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-unconfirmed help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants