-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Allow using precompiled shaders in Metal backend #8814
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
base: main
Are you sure you want to change the base?
Conversation
Is there any reason we wouldn't want to make this the default behavior? |
Regarding documentation, doc/Metal.md would be a natural place to put this. |
I think it can't be the default because it rules out cross compilation. |
The other question I have is whether using this option raises any sort of compatibility concerns such that it should be a target option? |
I wanted to make this strictly opt-in, given it doesn't work with cross-compilation, and not invisible/automatic because users who want compiled shaders likely want to know if compilation failed for whatever reason.
I haven't been able to think of a scenario where this might happen. Apple's Metal IR/metallib seems to be intermediate formats compatible with any Apple-supported GPU. Shaders in text from can target both iOS and macOS, and it's unclear whether a single metallib will work on both (since the SDK is specified when calling the Metal compiler), but this isn't a real concern given we already generate CPU-side code that isn't portable across the two platforms. |
The hannk failures are fixed in #8815 (not yet merged) |
For security reasons, I'm a little uneasy doing environment variable lookups with no way to disable them at all. E.g. if someone is using Halide as JIT in a process, an attacker could arrange for the env vars to be set and thus hijack compilation. (It also might be difficult to set them in an application when one does want to use them, but I'm less concerned about that because at least in that a case the developer is aware they exist.) A potential solution is to call |
Adding to @zvookin's point... The values of the environment variables in this PR each reach a call to We overuse environment variables in this codebase, anyway. For example, I only recently discovered we can pass arbitrary extra flags to LLVM's options parser via Is there anything more limited we can do here? For instance, it might be acceptable to sniff the target to determine the correct SDK to call for |
I've raised this issue before and I was told that we don't want to document them because doing so would enshrine them as something other than hacks.
We inconsistently use
I'm sure @zvookin could refine this sketch into something better. |
To move this specific case forwards without shaving a herd of yaks, maybe they could be globals in libHalide with a setter. If someone really wants them driven by env vars they can write:
in their JIT compiler or AOT generator. globals have downsides, but they're no worse than env vars. It's hard to imagine a context where you want to use a different metal compiler/linker per compilation within the same process. |
This PR makes it possible to specify two environment variables
HL_METAL_COMPILER
andHL_METAL_LINKER
to make it possible for Halide to compile Metal shaders into embedded.metallibs
(Apple's libraries for Metal code). For example:This works for AOT code as well.
Looking for feedback on where to document this, and any other concerns folks may have. Feedback on how to test on the buildbots is also very welcome.