Skip to content

Conversation

slaren
Copy link
Member

@slaren slaren commented Aug 7, 2024

Should fix #8211

In some cases, the assembler seems to build the object for the wrong architecture. cmake uses the C compiler to build this file, so doing the same in the Makefile may fix it.

Makefile Outdated
@echo ".globl _ggml_metallib_end" >> $(TEMP_ASSEMBLY)/ggml-metal-embed.s
@echo "_ggml_metallib_end:" >> $(TEMP_ASSEMBLY)/ggml-metal-embed.s
$(CC) $(CFLAGS) -c $(TEMP_ASSEMBLY)/ggml-metal-embed.s -o $@
@rm -fr ${TEMP_ASSEMBLY}
Copy link
Member Author

Choose a reason for hiding this comment

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

Hopefully this will not turn into rm -fr / if something goes wrong, but I am open to suggestions for a better way to do this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does a simple string length check work? (I'm not sure if the code below is valid in makefile)

$( [ "$TEMP_ASSEMBLY" -lt 6 ] && exit 1 )

Copy link
Member Author

Choose a reason for hiding this comment

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

I have split this into a rm to delete the file, and a rmdir to delete the directory. This way, we don't need to use -r with rm, which should be a bit safer.

@slaren slaren merged commit 15fa07a into master Aug 7, 2024
54 checks passed
@slaren slaren deleted the sl/fix-metal-embd-build branch August 7, 2024 16:24
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
* make : use C compiler to build metal embed object

* use rm + rmdir to avoid -r flag in rm
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 this pull request may close these issues.

Bug: ld: symbol(s) not found for architecture arm64
3 participants