Repository navigation
Build the CUDA backend outside fbcode, without the LLM cache - #24817
mattiasbrand wants to merge 2 commits into
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24817
Note: Links to docs will display an error until the docs builds have been completed. ❌ You can merge normally! (4 Unrelated Failures), 4 Unclassified FailuresAs of commit 2658663 with merge base 618abb8 ( UNCLASSIFIED FAILURES - DrCI could not classify the following jobs because the workflow did not run on the merge base. The failures may be pre-existing on trunk or introduced by this PR:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
|
@mattiasbrand has exported this pull request. If you are a Meta employee, you can view the originating Diff in D129654471. |
This PR needs a
|
b41b43f to
251ee69
Compare
…ytorch#24817) Summary: Pull Request resolved: pytorch#24817 The CUDA backend's C++ runtime builds only in fbcode, and only with the LLM extension's KV cache linked in. So a C++ application that runs an exported vision or audio model on an NVIDIA GPU can't use it from any other Buck cell, cross-compiled aarch64 targets such as Jetson included. To run the same AOTInductor-compiled kernels it has to link LibTorch instead. XNNPACK, Vulkan and QNN already define their C++ targets in both cells; this does the same for CUDA. - **Targets in both cells.** The `aoti/slim`, `extension/cuda` and `cuda/runtime` C++ targets are now defined outside fbcode too. There they carry `compatible_with = ["ovr_config//cuda:has_cuda"]`, so CPU-only builds skip them instead of failing to compile, and SlimTensor `storage` picks up its CUDA deps through a `select`. The Python AOTI targets stay fbcode-only. - **Public headers.** Headers that dependents include (`slim_tensor.h`, `storage.h`, `runtime_api.h`, the shim and factory headers) move from `headers` to `exported_headers`, and `runtime_api`'s CUDA dependency becomes an exported external dep. Outside fbcode `headers` are private to the target, so nothing could compile against them. fbcode already exports both, so nothing changes there. - **LLM cache optional, as in CMake.** CMake already builds `cuda_kv_cache.cpp` and `cuda_kv_pool.cpp` only with `EXECUTORCH_BUILD_EXTENSION_LLM`. Buck now does the same outside fbcode: the cache code in `cuda_backend.cpp` and `cuda_kv_cache.h` is behind that define, and the define is exported so that dependents see the same declarations as the backend. CMake now exports it too (`PUBLIC` instead of `PRIVATE` on `aoti_cuda_backend`), since the header's declarations depend on it; `test_cuda_kv_cache` and `cuda_batching` include that header. fbcode keeps the cache. A backend built without it still refuses a program whose KV cache lives off-graph, because `requires_offgraph_kv_storage` reads that from the program's constants and sits outside the gate. - **Build flags.** The `nvcc` architecture flags stay fbcode-only. Outside fbcode the shims undefine `CPU_CAPABILITY_AVX2` and `CPU_CAPABILITY_AVX512` for CUDA sources: c10 exports them on x86, and `Half.h` then pulls host AVX2 `vec_half` code into nvcc compiles. `cuda-lazy` maps to `libcudart`, and the shims link `curand` and `thrust` for `rand.cu` and `sort.cu`. fbcode builds keep the same sources and flags, and the CMake build is untouched. Differential Revision: D129654471
|
@claude /pr-review |
|
Claude finished @Gasoonjia's task in 44m 22s —— View job Request Changes: this builds the CUDA backend in the xplat cell, but most of the newly exposed CUDA targets lack the Detailed reviewPR Review: #24817SummaryThis PR defines the Infrastructure
API Design
Testing
Code Quality
RecommendationRequest Changes The point of the PR is to let a Buck cell outside fbcode link the CUDA backend. That cell currently gets six CUDA targets with no CPU-only guard, which can break wildcard and CPU builds. It also gets a link requirement that every consumer has to rediscover. Please fix those two, and confirm or rule out the OSS
|
| #include <executorch/backends/cuda/runtime/backend_options.h> | ||
| #include <executorch/backends/cuda/runtime/cuda_delegate_handle.h> | ||
| #include <executorch/extension/cuda/runtime_api.h> | ||
| #if defined(EXECUTORCH_BUILD_EXTENSION_LLM) |
There was a problem hiding this comment.
EXECUTORCH_BUILD_EXTENSION_LLM is must-have for cuda_kv_cache and our cmake is alreaady guard it.
I think we should:
- follow the same way in buck target
- In the places including
cuda_kv_cache.hlikecuda_backend.h, we useEXECUTORCH_BUILD_EXTENSION_LLMmarco to guard it.
Summary: Pull Request resolved: pytorch#24643 Make the Buck CUDA runtime link its AOTI shims the way the CMake build already does. A compiled AOTInductor shared library does not contain the tensor runtime. When the CUDA backend loads it, the library resolves its `aoti_torch_*` shim calls from the host process by name. Nothing in the host code calls most of these functions directly, so a static link without whole-archive drops them from `common_shims_slim` and loading fails with unresolved symbols. CMake already links `aoti_common_shims_slim` with `--whole-archive`; this sets `link_whole = True` on the Buck target to match, along with `supports_python_dlopen = True` like the other force-linked AOTI and CUDA targets. An executable that loads an AOTI library also has to export these symbols, as CMake does with `-Wl,--export-dynamic`. That includes a Buck `python_binary` that uses the Python bindings, because the extension links into its native executable. `backends/aoti/tests:test_common_shims_slim_link` looks every shim up by name in a statically linked test binary that never calls them. It fails for every shim when `link_whole` is removed. `test_common_shims_slim` cannot catch this, because it calls the shims directly and that keeps them linked. `backends/cuda/runtime/shims/tensor_attribute.cpp` defined `aoti_torch_get_device_type` and `aoti_torch_device_type_cuda` for ETensor, which the CUDA backend no longer uses. `common_shims_slim` defines the same `extern "C"` symbols for SlimTensor, so once both are force-linked they collide. CMake already leaves this file out of the CUDA shim sources. This deletes the file and its header and removes them from the Buck target. `executorch_pybindings(use_cuda = True)` compiles code that calls the CUDA runtime directly but did not declare the CUDA dependency, so such a target could not link. The macro now adds it when `use_cuda` is set, together with `//executorch/extension/cuda:cuda_allocator`, which `pybindings_cuda.h` includes and calls. Previously that dependency was only met indirectly through the AOTI CUDA backend. `//executorch/extension/pybindings:cuda_lib` builds these bindings with the AOTI CUDA backend. LibTorch defines the same `aoti_torch_*` symbols with a different tensor handle ABI, so it cannot link into the same binary; the target says so, and is visible only to the pybindings packages. OSS Buck has no CUDA backend targets and no mapping for the CUDA dependency, so `executorch_pybindings(use_cuda = True)` defines nothing there; the CMake build links CUDA into `portable_lib` instead. `extension/pybindings/test:test_pybindings_cuda_lib` exports a CUDA-delegated program with external weights, runs it through `cuda_lib` in a separate LibTorch-free binary, and compares the output with eager PyTorch. Without `link_whole`, or without `-Wl,--export-dynamic` on that binary, it fails with `undefined symbol: aoti_torch_dtype_float32`. `pytest.ini` and `pytest-windows.ini` leave it out of the pytest runs, because it needs a GPU and the Buck-built runner. Differential Revision: D129530257
Summary: The CUDA backend's C++ runtime builds only in fbcode, and only with the LLM extension's KV cache linked in. So a C++ application that runs an exported vision or audio model on an NVIDIA GPU can't use it from any other Buck cell, cross-compiled aarch64 targets such as Jetson included. To run the same AOTInductor-compiled kernels it has to link LibTorch instead. XNNPACK, Vulkan and QNN already define their C++ targets in both cells; this does the same for CUDA. - **Targets in both cells.** The `aoti/slim`, `extension/cuda` and `cuda/runtime` C++ targets are now defined outside fbcode too. There every CUDA target carries `platforms = [CXX]` and `compatible_with = ["ovr_config//cuda:has_cuda"]`, so CPU-only builds, including wildcard builds of these packages, skip them instead of failing to find `cuda_runtime.h`. SlimTensor `storage` picks up its CUDA deps through a `select`. The Python AOTI targets stay fbcode-only. - **Public headers.** Headers that dependents include (`slim_tensor.h`, `storage.h`, `runtime_api.h`, the shim and factory headers) move from `headers` to `exported_headers`, and `runtime_api`'s CUDA dependency becomes an exported external dep. Outside fbcode `headers` are private to the target, so nothing could compile against them. fbcode already exports both, so nothing changes there. - **LLM cache gated as in CMake.** CMake builds `cuda_kv_cache.cpp` and `cuda_kv_pool.cpp` only with `EXECUTORCH_BUILD_EXTENSION_LLM`. Buck now does the same: outside fbcode the backend builds without those sources, their headers and the `kv_cache` dependency. Only fbcode defines the macro, because only fbcode builds the LLM extension. `cuda_kv_cache.h` stays an LLM-only header. `cuda_backend.cpp` includes it, and uses the off-graph KV cache, only when the macro is defined. `requires_offgraph_kv_storage` reads the program's constants, not the cache, so it moves to `cuda_delegate_handle.h` and runs in every build. A backend built with the LLM cache still asks for a cache under `llm_cache_registry_key`. One built without it refuses a program whose KV cache lives off-graph with `NotSupported`, and ignores a cache key passed for any other program. - **Linking.** A compiled AOTI library binds its `aoti_torch_*` calls to the shims in the host executable. CMake makes that work with a `PUBLIC` `--export-dynamic` on `aoti_cuda_backend`. Outside fbcode, `cuda_backend` now exports the same flag on Linux, so an executable that links it statically can load a model without adding the flag itself. - **Build flags.** The `nvcc` architecture flags stay fbcode-only; outside fbcode the CUDA toolchain picks the architecture. Outside fbcode the shims also undefine `CPU_CAPABILITY_AVX2` and `CPU_CAPABILITY_AVX512` for CUDA sources: c10 exports them on x86, and `Half.h` then pulls host AVX2 `vec_half` code into nvcc compiles. `cuda-lazy` maps to `libcudart`, and the shims link `curand` and `thrust` for `rand.cu` and `sort.cu`. fbcode builds keep the same sources, flags and LLM cache. In a CMake build with the LLM extension, behavior is unchanged. In a CMake build without it, a cache key passed for a program that doesn't keep its KV cache off-graph is now ignored rather than rejected, as a backend with the cache already ignores it for such a program. Differential Revision: D129654471
251ee69 to
2658663
Compare
meta-codesync
Bot
changed the title
Summary:
The CUDA backend's C++ runtime builds only in fbcode, and only with the LLM extension's KV cache linked in. So a C++ application that runs an exported vision or audio model on an NVIDIA GPU can't use it from any other Buck cell, cross-compiled aarch64 targets such as Jetson included. To run the same AOTInductor-compiled kernels it has to link LibTorch instead. XNNPACK, Vulkan and QNN already define their C++ targets in both cells; this does the same for CUDA.
aoti/slim,extension/cudaandcuda/runtimeC++ targets are now defined outside fbcode too. There every CUDA target carriesplatforms = [CXX]andcompatible_with = ["ovr_config//cuda:has_cuda"], so CPU-only builds, including wildcard builds of these packages, skip them instead of failing to findcuda_runtime.h. SlimTensorstoragepicks up its CUDA deps through aselect. The Python AOTI targets stay fbcode-only.slim_tensor.h,storage.h,runtime_api.h, the shim and factory headers) move fromheaderstoexported_headers, andruntime_api's CUDA dependency becomes an exported external dep. Outside fbcodeheadersare private to the target, so nothing could compile against them. fbcode already exports both, so nothing changes there.cuda_kv_cache.cppandcuda_kv_pool.cpponly withEXECUTORCH_BUILD_EXTENSION_LLM. Buck now does the same: outside fbcode the backend builds without those sources, their headers and thekv_cachedependency. Only fbcode defines the macro, because only fbcode builds the LLM extension.cuda_kv_cache.hstays an LLM-only header.cuda_backend.cppincludes it, and uses the off-graph KV cache, only when the macro is defined.requires_offgraph_kv_storagereads the program's constants, not the cache, so it moves tocuda_delegate_handle.hand runs in every build. A backend built with the LLM cache still asks for a cache underllm_cache_registry_key. One built without it refuses a program whose KV cache lives off-graph withNotSupported, and ignores a cache key passed for any other program.aoti_torch_*calls to the shims in the host executable. CMake makes that work with aPUBLIC--export-dynamiconaoti_cuda_backend. Outside fbcode,cuda_backendnow exports the same flag on Linux, so an executable that links it statically can load a model without adding the flag itself.nvccarchitecture flags stay fbcode-only; outside fbcode the CUDA toolchain picks the architecture. Outside fbcode the shims also undefineCPU_CAPABILITY_AVX2andCPU_CAPABILITY_AVX512for CUDA sources: c10 exports them on x86, andHalf.hthen pulls host AVX2vec_halfcode into nvcc compiles.cuda-lazymaps tolibcudart, and the shims linkcurandandthrustforrand.cuandsort.cu.fbcode builds keep the same sources, flags and LLM cache. In a CMake build with the LLM extension, behavior is unchanged. In a CMake build without it, a cache key passed for a program that doesn't keep its KV cache off-graph is now ignored rather than rejected, as a backend with the cache already ignores it for such a program.
Differential Revision: D129654471