-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
Metal support for Swift #3078
Metal support for Swift #3078
Changes from 2 commits
89a96fd
ce92d75
99f85e7
690c794
24013b3
d58af4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,23 +4,27 @@ import PackageDescription | |
|
||
let package = Package( | ||
name: "llama", | ||
platforms: [.macOS(.v11)], | ||
products: [ | ||
.library(name: "llama", targets: ["llama"]), | ||
], | ||
targets: [ | ||
.target( | ||
name: "llama", | ||
path: ".", | ||
exclude: ["ggml-metal.metal"], | ||
sources: [ | ||
"ggml.c", | ||
"llama.cpp", | ||
"ggml-alloc.c", | ||
"k_quants.c" | ||
"k_quants.c", | ||
"ggml-metal.m", | ||
], | ||
publicHeadersPath: "spm-headers", | ||
cSettings: [ | ||
.unsafeFlags(["-Wno-shorten-64-to-32"]), | ||
.unsafeFlags(["-fno-objc-arc"]), | ||
.define("GGML_SWIFT"), | ||
.define("GGML_USE_METAL"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this change forcing everyone to use metal? Can this be a flag defined by the customer instead? For instance, I can't run llamacpp with metal ON with my old MacBook and an AMD card There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kchro3 Let's address this comment and we can merge There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how does it look now? i don't have an old macbook, but i was able to build it if i switched the if/else condition
|
||
.define("GGML_USE_K_QUANTS"), | ||
.define("GGML_USE_ACCELERATE") | ||
], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,12 +141,19 @@ @implementation GGMLMetalClass | |
|
||
ctx->d_queue = dispatch_queue_create("llama.cpp", DISPATCH_QUEUE_CONCURRENT); | ||
|
||
#if 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thought it would be ok to replace this since it looked unfinished. |
||
// compile from source string and show compile log | ||
#ifdef GGML_SWIFT | ||
// load the default.metallib file | ||
{ | ||
NSError * error = nil; | ||
|
||
ctx->library = [ctx->device newLibraryWithSource:msl_library_source options:nil error:&error]; | ||
NSBundle * bundle = [NSBundle bundleForClass:[GGMLMetalClass class]]; | ||
NSString * llamaBundlePath = [bundle pathForResource:@"llama_llama" ofType:@"bundle"]; | ||
NSBundle * llamaBundle = [NSBundle bundleWithPath:llamaBundlePath]; | ||
NSString * libPath = [llamaBundle pathForResource:@"default" ofType:@"metallib"]; | ||
|
||
// Load the metallib file into a Metal library | ||
ctx->library = [ctx->device newLibraryWithFile:libPath error:&error]; | ||
|
||
if (error) { | ||
metal_printf("%s: error: %s\n", __func__, [[error description] UTF8String]); | ||
return NULL; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tried iOS but I do wonder if we are OK by limiting this package to macOS only. Can llamacpp run on iOS, tvOS or even watchOS @ggerganov?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can run even on a refrigerator 😄
Jokes aside - I see no reason to limit this to just macOS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not sure why, but the compiler was not happy if platforms was not included, although that could be a metal thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't have a way to test on watchOS or tvOS... i can set it to the minimum non-deprecated version and if someone tries to cross that bridge, we could update it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mind sharing the compiler error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey folks, I would appreciate help on this. I'm seeing in my build logs that it's compiling the .metal file even if I put it in
resources
.For example, if I do:
I still see the default.metallib in my resources:
Could it be because the file is in the project root and the target path is "."?tried copying it into a new directory & excluding the original, but it still compiled...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if you exclude ggml-metal.metal like it is done in master right now, but then add it in the resources section. That should do it I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://developer.apple.com/documentation/packagedescription/target/exclude#discussion
I think it doesn't work because exclude takes precedence over resources. For example, I pushed a branch https://github.com/ggerganov/llama.cpp/pull/3091/files, and my build logs don't show the .metal file:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://developer.apple.com/library/archive/documentation/3DDrawing/Conceptual/MTLBestPracticesGuide/FunctionsandLibraries.html
Just a sanity check, but this documentation is saying that .metal files get automatically compiled, and that seems to be the case from what I've tried. Is there a specific reason why we need to compile from source?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, it looks like a better way if we use it on Xcode.