Ship the CUDA delegate in the wheel - #21645
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21645
Note: Links to docs will display an error until the docs builds have been completed. ❌ 31 New Failures, 1 Unrelated Failure, 31 Unclassified FailuresAs of commit 80c9986 with merge base ed65b12 ( NEW FAILURES - The following jobs have failed:
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:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
| // this interface needs. Repeating the declaration CUDA itself makes is | ||
| // well-formed, so a consumer that also includes <cuda_runtime.h> is unaffected, | ||
| // in either include order. | ||
| typedef struct CUstream_st* cudaStream_t; |
There was a problem hiding this comment.
is this the only thing we need from the toolkit
| "aoti_torch_cuda_rand", | ||
| "aoti_torch_cuda_randint_low_out", | ||
| ) | ||
|
|
There was a problem hiding this comment.
can we also check for our local cuda kernels?
| # CUDA index. A CPU wheel defines neither, so a consumer asking for one is told | ||
| # while configuring. | ||
| _executorch_define_component(backend_cuda executorch_backend_cuda) | ||
| _executorch_define_component(extension_cuda executorch_extension_cuda) |
There was a problem hiding this comment.
I guess this is usable only when toolkit is installed?
digantdesai
left a comment
There was a problem hiding this comment.
Thanks, skimmed it, sorry going slow..
Yes, and I checked rather than assumed. Here is the evidence. The header uses exactly one CUDA name. Grepping every CUDA identifier in the file gives only EXECUTORCH_EXTENSION_CUDA_API std::optional<cudaStream_t> getCallerStream();
static_assert(std::is_trivially_copyable_v<std::optional<cudaStream_t>>);The stream is only stored and handed back, never dereferenced, so an opaque handle is all the It compiles with no toolkit at all, which is the point of the change: the wheel ships this The declaration matches CUDA's own exactly, so there is no risk of a subtly different type. typedef __device_builtin__ struct CUstream_st *cudaStream_t;
#include <cuda_runtime.h>
#include <executorch/extension/cuda/caller_stream.h>
static_assert(std::is_same_v<cudaStream_t, struct CUstream_st*>);That compiles, so the two declarations are the same type rather than merely similar. Both include orders work. Repeating a typedef is well formed in C++, so a consumer that also If this interface ever needs to touch a stream rather than pass it along, for example to |
|
Two more from this round.
|
The CUDA delegate runs a model on an NVIDIA GPU. It is built into the Python extension, so only
Python can use it. A C++ application has no way to link it, and nothing else can reuse it either.
There is a sharing problem too. A program may use more than one GPU backend at once, and they need
to agree on which CUDA stream (the queue the GPU runs work on) the caller chose. If each backend
carries its own copy of that state, work queued through one is invisible to the other.
Ship the CUDA delegate and a small stream helper as their own shared libraries, and name both as
CMake components. The stream helper is shared so a process has exactly one copy of the caller's
stream choice, which is what lets two backends agree on it.
A CUDA wheel does not bundle the CUDA runtime. It declares it as a dependency, the way the PyTorch
CUDA wheels do, so one copy is shared with torch rather than shipping a second one:
Each library records a relative path to where pip installs the CUDA runtime, so it resolves without
the caller setting a library search path and without depending on a toolkit being installed.
The declared set includes the runtime compiler, because a shipped library links it to build kernels
at run time. Each declared package also needs its own directory recorded, since that is where the
loader looks. On the CUDA 12 packaging the compiler installs into its own directory, and omitting it
left that library unable to find the compiler even though the package was installed. The CUDA 13
packaging puts every component in one directory, so the same gap does not appear there.
The stream helper's header no longer includes
cuda_runtime.h, which the wheel does not publish. Itonly ever uses a CUDA stream as an opaque handle, so it declares that handle itself, and a consumer
can compile against the wheel with no CUDA toolkit installed.
Built a CUDA wheel, installed it into a clean environment, and:
same weights and inputs (largest absolute difference 0).
same reference.
into every consumer put three copies in one wheel, and a stream selected through one was invisible
to the others.
that links the CUDA runtime has a relative path to it.
so a builder without a matching toolkit fails while configuring. Before this, such a row produced a
wheel tagged for CUDA, carrying no CUDA library, that still declared the CUDA runtime packages: it
installed cleanly and then reported the backend as unregistered when a model ran.
declared packages and the loader paths come from the row while the binaries come from the
toolkit, and nothing compared the two, so a
cu126row built against a 13.0 toolkit attachedCUDA 12 metadata to binaries needing
libcudart.so.13. An unrecognised train fails too, insteadof silently reporting whatever the builder happened to have. Detection reads the toolkit major
directly, so the guard fires on any mismatch rather than only on the three exact
(major, minor)pairs the supported list carries; on those three pairs it behaved correctly before, and on every
other minor it saw an empty detection and skipped the check.
spelled with an unsupported minor means. The shell classifier reduces the row to digits and
matches against
SUPPORTED_CUDA_VERSIONS. Packaging did the same shape on the outer decisionand then took only the first two digits when picking runtime packages, so
cu125classified asCPU on one side and declared CUDA 12 on the other. Packaging now matches on the same digits and
raises loudly on an unsupported train instead.
listing the spellings that mean "no CUDA". Checked 16 row values including
cpu-aarch64,rocm6.2and
cu118; the previous list-based form was wrong on several, and each wrong answer made anon-CUDA wheel declare the CUDA runtime.
optional unconditionally, so a wheel tagged
+cu126with no CUDA library at all passed every check.dependency whenever CUDA is on, while packaging named only the shared-build spelling, so a
non-shared build shipped a shim whose dependency resolved to nothing.
A fixed pair was correct at one depth only: measured over every shipped location, 6 of 12 hops
landed on a directory that does not exist, and the hop from
lib/climbed out of the packageentirely, where an unrelated library with a matching soname could satisfy the dependency first.
opaque handle and calls no CUDA function, so it needs no toolkit include and no libcudart link.
Also fixed in this commit:
python3orpython, whicheverexists) instead of assuming one name, and no longer discards stderr. Builders disagree on the
name: Linux and macOS provide
python3, while the Windows builder runs inside a condaenvironment that provides only
python. Assuming either name breaks the other platform, andtreating the failure as "not a CUDA row" silently rebuilt a CUDA row as a CPU row.
CU_VERSION=cpu pip install .is handled explicitly instead of running the CUDA-train parserover it, which previously turned
cpuintoputhrough a character-set strip and reached theunsupported-train error.
Ran end to end on H100, A100 and Jetson Thor, covering compute capabilities 9.0, 8.0 and 11.0.
Known gap, not introduced here: the Python
Runtime.load_programpath allocates activation memoryon the host, so a program exported to keep activations on the GPU fails there. The supported Python
loader and the C++ path both work. This is upstream in the Python bindings, which this change does
not touch.