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

fix makefile and cmake logic for AARCH64 #11246

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Haus1
Copy link

@Haus1 Haus1 commented Jan 15, 2025

Some ifndef slipped through in place of ifdefs and are throwing the compiler for a loop.

Use ifdef to match inclusive filters in the Makefile and have GGML_CPU_AARCH64 default to OFF. This prevents the compiler from becoming confused and optimizing for the wrong architecture.

#11204
#11082
#11083
#11079
#11071
#10933

Some ifndef slipped through in place of ifdefs and are throwing the compiler for a loop.

Use ifdef to match inclusive filters in the Makefile and have GGML_CPU_AARCH64 default to OFF.
This prevents the compiler from becoming confused and optimizing for the wrong architecture.
@github-actions github-actions bot added the ggml changes relating to the ggml tensor library for machine learning label Jan 15, 2025
@slaren
Copy link
Collaborator

slaren commented Jan 16, 2025

GGML_CPU_AARCH64 enabled by default is not a bug. The name of the feature is misleading, but it also supports AVX2.

@Haus1
Copy link
Author

Haus1 commented Jan 18, 2025

Is this in addition to what's enabled with GGML_AVX2?

Part of the motivation behind this is the build failing for clang versions prior to 18, specifically the ROCM compiler. These changes fall short of fixing it but everything I've found points to it being an issue with clang's handling of aarch64 and getting stuck in an infinite loop.

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