Add WebGPU dependencies and refactor *_TO_BUILD CMake options.#7905
Add WebGPU dependencies and refactor *_TO_BUILD CMake options.#7905ScottTodd merged 3 commits intoiree-org:mainfrom
Conversation
ScottTodd
force-pushed
the
webgpu-deps
branch
from
943b013 to
a7c9369
Compare
| ], | ||
| ) | ||
|
|
||
| iree_cmake_extra_content( |
There was a problem hiding this comment.
I wish we had a better way of doing this - like a custom field on the rule that was like cmake_enable_if = "${IREE_HAL_DRIVER_VMVX}" or something - these blocks are so ugly ;(
There was a problem hiding this comment.
Yeah :P we've used this particular escape hatch in this way enough times that it would be worth promoting it to something with its own syntax.
| set(IREE_HAL_DRIVERS_TO_BUILD ${IREE_ALL_HAL_DRIVERS}) | ||
|
|
||
| # For Apple platforms we need to use Metal instead of Vulkan. | ||
| # Vulkan is not natively supported on Apple platforms. |
There was a problem hiding this comment.
This logic always sketched me out as it obfuscates both the name of the cmake options we're setting and requires people to evaluate the cmake in their head to know what's all/default/enabled
It'd be nice to have real actual explicit cmake options (like tint has TINT_BUILD_GLSL_WRITER and such) - every time I see an if(${IREE_HAL_DRIVER_FOO}) and try to search for it I get sad when I remember we're doing dynamic variable name construction. I think it was a tradeoff for making one way of setting things (a delimited string flag on the command line) easier while making everything else way harder to reason about. We can still have the string parsing for the simplified command line but having them backed by concrete cmake options feels like it'd be much less magic. It'd also make it easier to have more of these kind of settings in the future (all, default, host, cpu, etc) that just become if(cpu) set(SOMETHING ON).
This PR is fine as it's just adding to the pile of sadness, but something to think about as we grow this - getting tint and dawn set up was pretty easy because they had nice clear options with no magic, I wouldn't want to set up iree ;P When embedding in other projects being able to set(IREE_HAL_DRIVER_FOO ON/OFF) is way more natural than a string.
There was a problem hiding this comment.
SG, I can refactor all those options in a future PR.
| add_subdirectory(third_party/spirv_cross EXCLUDE_FROM_ALL) | ||
| endif() | ||
|
|
||
| if(IREE_TARGET_BACKEND_WEBGPU) |
There was a problem hiding this comment.
(future cleanup, like whenever metal uses tint) we'll likely want a dedicated option for whether to enable SPIRV_TOOLING (or w/e) - it's easier to reason about "metal, d3d12, wgsl, and vulkan need SPIRV_TOOLING" by having if(d3d12) set(SPIRV_TOOLING ON) kind of blocks that are easy to search for/tweak than to have that if statement in all those places expanded out like we do in places today.
| set(LLVM_EXTERNAL_PROJECTS ${LLVM_EXTERNAL_PROJECTS} CACHE STRING "" FORCE) | ||
| endmacro() | ||
|
|
||
| macro(iree_set_spirv_headers_cmake_options) |
There was a problem hiding this comment.
I hate this file - it's the last place I'd look for settings despite the name - I really like having the settings near the FetchContents like we have with tint - maybe as we refactor things we can do that more. It also makes it easier to keep bazel and cmake in sync as the bzl and cmakelists live side-by-side instead of having all these things set in build_tools/cmake/. If we're able to fully exclude third_party/spirv_headers/ then maybe we switch that to FetchContent as well.
ScottTodd
force-pushed
the
webgpu-deps
branch
from
a7c9369 to
f8519ca
Compare
) Following up on #7905 (comment), this uses regular options for these settings, which are easier to search for and set. The CMakeLists from Tint was used as a reference: https://dawn.googlesource.com/tint/+/refs/heads/main/CMakeLists.txt. The main focus is to use explicit names instead of dynamic name construction, but using [option](https://cmake.org/cmake/help/latest/command/option.html) is a nice bonus. New `IREE_HAL_DRIVER_DEFAULTS` and `IREE_TARGET_BACKEND_DEFAULTS` options can be used to set only a specific subset (set to `OFF` by default, then enable just the features you want).
3 participants
Part of #7840
IREE_HAL_DRIVERS_TO_BUILDandIREE_TARGET_BACKENDS_TO_BUILDnow use "default" instead of "all".The new WebGPU compiler target is included in the list of all targets but is not enabled by default, and the Metal+CUDA targets similarly disable themselves on incompatible platforms. Developers must explicitly opt in to building the WebGPU compiler target or the Metal/CUDA targets in unsupported configurations. I'm using
-DIREE_TARGET_BACKENDS_TO_BUILD=CUDA;Dylib-LLVM-AOT;WASM-LLVM-AOT;Vulkan-SPIRV;VMVX;WebGPU, for example.Thanks to being conditionally enabled, we can also use CMake's FetchContent instead of git submodules for some dependencies, which will hopefully keep the rest of IREE's CMake and git infrastructure simple for most users while only adding costs for those us of working on or using these optional components.