Skip to content

Commit cff6f4d

Browse files
authored
Ship the CUDA Windows import library only in CUDA builds (pytorch#22054)
Depends on pytorch#22053, which is what makes a flag declared on a source tree file take effect. That one has landed and this branch is rebased on top of it. ### Summary `backends/cuda/runtime/aoti_cuda_shims.lib` is a Windows import library: the small link stub a Windows program links against so its calls reach the CUDA delegate's shim DLL at run time. Lowering for Windows is a cross compile, so a Linux CUDA build needs it too, not only a Windows build. Unlike the other files the wheel picks up, it is checked into the source tree rather than built, and its packaging entry declared no CMake options, so it went into every published wheel. On the most recent nightly cut that is all 50 rows: the 30 CUDA rows, which can use it, and the 20 CPU rows, which cannot. Five of those CPU rows are macOS, where the CUDA delegate cannot exist at all. Those wheels carry a link stub for a library they do not contain. Declare `EXECUTORCH_BUILD_CUDA` on the entry, the same option that gates the delegate's own shared library, which is the very next entry in `ext_modules`. The import library now ships when the delegate does and not otherwise. The code that reads it back needs a matching check, because this change is what makes that directory disappear. When lowering for a Windows target, `cuda_backend` defaults the shim library path to the installed package's `data/lib` directory. On a CPU wheel today the import library is the only file in there, so once it stops shipping, the directory stops existing. `executorch.data.lib` is not a declared package, so nothing else creates it, and a directory that is not there still reads back as an ordinary path instead of raising. Without a check, a CPU install would pass a missing directory straight into the AOTInductor link options and the failure would surface far from its cause. It now fails where the assumption breaks, and names both the fix and the `shim_library_path` compile spec that overrides the default. ### Test plan Called `get_aoti_compile_options()` with a `windows` platform compile spec against a package directory with no `data/lib`, one with an empty `data/lib`, one holding the import library, and one with no `data/lib` but an explicit `shim_library_path` spec. The first two raise and the last two return the expected path. Ran the same four without the new check and all four returned a path, including the one that does not exist. The new wheel test asks whether the wheel ships the `aoti_cuda_shims` shared library, not whether its version carries a `+cuXXX` label, because only a published wheel carries that label and a locally built CUDA wheel would otherwise be told to drop a file it needs. Both files are packaged behind `EXECUTORCH_BUILD_CUDA` alone, so they arrive together. Ran it in all four combinations of the shim library present or absent and the import library present or absent, and it passes only on the two that match. Ran it against today's published nightly wheels: the CPU wheel fails, which is the case this change removes, and the CUDA wheel passes. Removing the import library from the CPU wheel, which is what this change does, turns that failure into a pass and leaves the CUDA wheel passing. That test runs from the wheel test entry point used by the Linux, Linux aarch64, macOS and CUDA Linux jobs, so it covers 45 of the 50 wheels published today. The Windows job does not use that entry point, so the 5 Windows wheels are not covered by it. The same packaging change was built by the wheel jobs on pytorch#22049. Unzipping six of those wheels: the CPU, macOS and Windows wheels lose `executorch/data/lib/aoti_cuda_shims.lib` and nothing else, and the CUDA wheels are unchanged file for file.
1 parent 16fef19 commit cff6f4d

3 files changed

Lines changed: 49 additions & 1 deletion

File tree

‎.ci/scripts/wheel/test_shared_libraries.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2343,6 +2343,41 @@ def test_shipped_library_names_are_expected() -> None:
23432343
print(f"✓ {len(shipped)} shipped libraries have expected names and identities")
23442344

23452345

2346+
def test_windows_import_library_tracks_the_cuda_delegate() -> None:
2347+
"""The Windows import library ships exactly where the CUDA delegate does.
2348+
2349+
It is the link input the delegate uses when lowering for a Windows target, so a
2350+
wheel without the delegate has no use for it and a CUDA wheel cannot do that
2351+
lowering without it. Checked here rather than by the symbol tests above, because
2352+
it is an archive of import stubs rather than a shared object, so nothing that
2353+
scans shipped libraries sees it. It is also checked in rather than built, which
2354+
is how it came to ship in every wheel with nothing noticing.
2355+
2356+
Keyed on the shim library the wheel actually ships rather than on the version
2357+
label, because only a published wheel carries a +cuXXX local version and a
2358+
locally built CUDA wheel would otherwise be told to drop a file it needs. Both
2359+
files are packaged behind EXECUTORCH_BUILD_CUDA alone, so they arrive together.
2360+
"""
2361+
package_dir = _installed_package_dir()
2362+
import_library = package_dir / "data" / "lib" / "aoti_cuda_shims.lib"
2363+
shim_library = _library_file_name("libaoti_cuda_shims")
2364+
present = import_library.is_file()
2365+
if any(
2366+
path.name.startswith(shim_library)
2367+
for path in _shipped_shared_objects(package_dir)
2368+
):
2369+
assert present, (
2370+
f"this CUDA wheel ships no {import_library.name}, so lowering the "
2371+
"delegate for a Windows target has nothing to link against."
2372+
)
2373+
else:
2374+
assert not present, (
2375+
f"this wheel ships {import_library.name} but no CUDA delegate, so it "
2376+
"carries a link stub for a library it does not contain."
2377+
)
2378+
print(f"✓ {import_library.name} {'ships' if present else 'is absent'} as expected")
2379+
2380+
23462381
_PARITY_MODEL = '''
23472382
import json
23482383
import sys
@@ -2559,6 +2594,7 @@ def run_tests(work_dir: Path) -> None:
25592594
test_declared_dependencies_match_the_wheel_tag()
25602595
test_extension_contains_no_component()
25612596
test_shipped_library_names_are_expected()
2597+
test_windows_import_library_tracks_the_cuda_delegate()
25622598
test_shipped_libraries_load()
25632599
test_shipped_libraries_resolve_without_build_tree()
25642600
test_custom_op_compiles(work_dir)

‎backends/cuda/cuda_backend.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,16 @@ def get_aoti_compile_options(
801801

802802
if shim_library_path is None:
803803
lib_dir = resources.files("executorch").joinpath("data/lib")
804+
# Only a CUDA build ships the import library, and a package directory
805+
# that does not exist still reads back as an ordinary path rather than
806+
# raising, so without this the failure surfaces from the linker instead.
807+
if not lib_dir.joinpath("aoti_cuda_shims.lib").is_file():
808+
raise RuntimeError(
809+
"Lowering for Windows links against aoti_cuda_shims.lib, which "
810+
"only a CUDA build of executorch ships. Install a CUDA build, "
811+
"or pass a shim_library_path compile spec naming a directory "
812+
"that holds the import library."
813+
)
804814
shim_library_path = str(lib_dir)
805815
options.update(
806816
{

‎setup.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2390,11 +2390,13 @@ def run(self): # noqa C901
23902390
is_dynamic_lib=True,
23912391
dependent_cmake_flags=["EXECUTORCH_BUILD_KERNELS_LLM_AOT"],
23922392
),
2393+
# A Windows import library for the delegate's shim DLL. Lowering for
2394+
# Windows is a cross compile, so a Linux CUDA build needs it too.
23932395
BuiltFile(
23942396
src_dir="backends/cuda/runtime/",
23952397
src_name="aoti_cuda_shims.lib",
23962398
dst="executorch/data/lib/",
2397-
dependent_cmake_flags=[],
2399+
dependent_cmake_flags=["EXECUTORCH_BUILD_CUDA"],
23982400
),
23992401
BuiltFile(
24002402
src_dir="%CMAKE_CACHE_DIR%/backends/cuda/%BUILD_TYPE%/",

0 commit comments

Comments
 (0)