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

cmake : use ggml-metal.metal from source dir to build default.metallib #9325

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

Conversation

cebtenzzre
Copy link
Collaborator

I noticed an edge case while building the ggml-metal target with Ninja more than once. The specific steps to reproduce are:

$ cmake -B build -G Ninja -DGGML_METAL_EMBED_LIBRARY=OFF  # copies ggml-metal.metal to build/bin/ 
$ ninja -C build ggml-metal  # succeeds, removes build/bin/ggml-metal.metal
$ ninja -C build clean  # removes generated default.metallib, but ggml-metal.metal is still gone
$ ninja -C build ggml-metal  # fails to find build/bin/ggml-metal.metal
ninja: Entering directory `build'
[1/1] Compiling Metal kernels
FAILED: bin/default.metallib /Users/jared/src/forks/llama.cpp/build/bin/default.metallib
cd /Users/jared/src/forks/llama.cpp/build/ggml/src && xcrun -sdk macosx metal -O3 -c /Users/jared/src/forks/llama.cpp/build/bin/ggml-metal.metal -o /Users/jared/src/forks/llama.cpp/build/bin/ggml-metal.air && xcrun -sdk macosx metallib /Users/jared/src/forks/llama.cpp/build/bin/ggml-metal.air -o /Users/jared/src/forks/llama.cpp/build/bin/default.metallib && rm -f /Users/jared/src/forks/llama.cpp/build/bin/ggml-metal.air && rm -f /Users/jared/src/forks/llama.cpp/build/bin/ggml-common.h && rm -f /Users/jared/src/forks/llama.cpp/build/bin/ggml-metal.metal
metal: error: no such file or directory: '/Users/jared/src/forks/llama.cpp/build/bin/ggml-metal.metal'
metal: error: no input files
ninja: build stopped: subcommand failed.

This does not happen with make, since it notices the missing files and runs cmake again to regenerate the byproducts (which are created at configure time via configure_file yet deleted at build time). This still happens after this PR, which is not ideal, but I don't fully understand why we are deleting build artifacts based on the target you select. @ggerganov Are the rm commands important here?

This also does not happen with the default of GGML_METAL_EMBED_LIBRARY=ON, since it does not rm the input files afterwards.

The simple fix is to use ggml-metal.metal from the source dir to build default.metallib, which exists regardless of whether you have previously run the target.


@cebtenzzre cebtenzzre requested a review from ggerganov September 5, 2024 16:39
@ggerganov
Copy link
Owner

@ggerganov Are the rm commands important here?

I am not really sure what was the reason to delete the files. Maybe we should remove the rm statements and see if it works as expected.

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.

2 participants