Conversation
Introduce a CUDA_PYTHON_TOOLCHAIN build-time env var (mirroring CUDA_PYTHON_PARALLEL_LEVEL) that switches the C/C++ compiler and linker as a unit. Valid values: gnu (default on Linux), llvm (clang + lld) on Linux, msvc (default on Windows). The defaults reproduce the previous build behavior exactly and do not touch CC/CXX, so an externally-set compiler (e.g. the sccache wrapper used in CI) keeps working. Each build_hooks.py gains two helpers: - _resolve_toolchain(): reads CUDA_PYTHON_TOOLCHAIN, validates it against the platform's allowed set, and returns the toolchain name plus its cc/cxx and extra_compile_args/extra_link_args. A non-default toolchain sets CC/CXX/LDSHARED so distutils' customize_compiler picks up clang/lld. - _check_toolchain_available(): a preflight that probes the toolchain's tools on PATH and fails fast with a helpful message (tool name, install hint, and how to fall back) instead of a cryptic compile error. cuda_bindings/setup.py drops the now-redundant _is_clang strip: the llvm flag set from _resolve_toolchain is clang-correct from the start (no -fpermissive, no -fno-var-tracking-assignments). No workflow changes; the default path composes with the existing hardcoded CC='sccache cc' in CI. CI toolchain selection and the CUDA_PYTHON_COMPILER_LAUNCHER companion var land in a follow-up.
mdboom
left a comment
There was a problem hiding this comment.
No real objection to this as-is, but it would be nice to reduce the duplication between the two build_hooks.py scripts somehow if possible.
I think in the long run, we will want to migrate to a more robust build backend, the top contender for which is probably scikit-build-core (based on CMake). In that environment, you wouldn't hardcode the flags to run explicitly, but have the configure step figure out which ones are available etc. My worry is that if we implement this now, we would have to duplicate this somehow inside of a better build system for backward compatibility and it may not make sense there. Would it be better to migrate now?
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
Removing the _is_clang handling breaks the externally supplied clang path that this PR says should keep working. When CUDA_PYTHON_TOOLCHAIN is unset, _resolve_toolchain() selects gnu and keeps the GCC-only -fno-var-tracking-assignments; because the default path intentionally does not override CC/CXX, a caller with CC=clang / CXX=clang++ now gets that unsupported flag. Previously build_ext detected clang and removed it. Please preserve compiler-based flag filtering on the default path (or resolve flags from the actual external compiler) and add a regression with externally supplied clang and no toolchain override.
Extract the toolchain logic that is identical across the two build_hooks.py files (constants, name resolution/validation, env application, preflight) into a single shared block delimited by 'begin/end shared toolchain helpers' markers, duplicated verbatim with a 'keep in sync' comment. This mirrors the existing precedent set by _import_get_cuda_path_or_home. The per-package _resolve_toolchain() is now a thin wrapper that calls the shared _resolve_toolchain_name()/_apply_toolchain_env() and assembles only its own package-specific flags (cuda.bindings: c++14, -fpermissive, -O3; cuda.core: c++17, -O2). The cc/cxx compiler mapping moves into a shared _TOOLCHAIN_COMPILERS table, so it is no longer re-assigned per branch. PEP 517 build isolation forbids a shared module (the sibling package is not installed in the isolated build env), so the block is duplicated rather than imported. A new test_shared_toolchain_block_is_in_sync enforces the byte-identical invariant so drift is caught locally. No behavior change: the defaults (gnu/msvc) reproduce the previous build flags exactly, and all existing build-hooks tests still pass.
b0289f6 to
67ac9fc
Compare
I tried extracting the shared toolchain helpers into a small distributable package (
FWIW there is precedent:
Hm, I don't think we promise backward compatibility for how the packages are built. Am I missing your point? |
…clang regression When CUDA_PYTHON_TOOLCHAIN is unset on Linux, infer the toolchain from the externally-set CC/CXX (CXX preferred, fall back to CC): a value containing 'clang' selects llvm, else gnu. This fixes the regression reported by sylvesterkaczmarek: previously the default 'gnu' flag set (incl. -fno-var-tracking-assignments) reached an externally-supplied clang because the default path did not override CC/CXX and the old _is_clang strip was removed. Now clang is inferred and the llvm flag set (no gcc-only flags, -fuse-ld=lld) is used, and the external compiler is left in place, so a wrapper like CC='sccache clang' survives and gets the llvm flags. When CUDA_PYTHON_TOOLCHAIN is set it takes precedence over an externally- set CC: a mismatch warns (CC only; CXX commonly defaults to 'c++' and is not a reliable user-intent signal) and the external CC is overridden. The shared toolchain helpers remain byte-identical across the two build_hooks.py via the 'keep in sync' markers. _resolve_toolchain_name now returns an 'explicit' flag so _apply_toolchain_env only overrides CC/CXX when the toolchain was chosen explicitly (not inferred). Tests: regression tests for externally-supplied CC=clang (infer llvm, no gcc-only flags, CC survives), CC=gcc (infer gnu), and the explicit mismatch/no-mismatch cases. A module-level autouse fixture cleans CC/CXX/LDSHARED per test because _apply_toolchain_env sets them directly in os.environ, which monkeypatch does not revert.
67ac9fc to
f8245a5
Compare
|
/ok to test f8245a5 |
…om dedup)
The dedup refactor accidentally changed gcc's '-std=c++17' (equals) to
'-std:c++17' (colon) in cuda_core/build_hooks.py's gnu and llvm
branches. gcc rejects the colon form ('unrecognized command-line
option'), breaking all Linux gcc builds of cuda.core (the pixi
smoke build and the linux-aarch64 wheel builds). cuda.bindings was
unaffected (it uses -std=c++14, correctly). Restore the equals form.
|
/ok to test 8ce0a8b |
|
Description
closes
Introduces a
CUDA_PYTHON_TOOLCHAINbuild-time override (mirroringCUDA_PYTHON_PARALLEL_LEVEL) that switches the C/C++ compiler and linker as a unit, without naming them explicitly. Valid values:gnu(default on Linux),llvm(clang + lld) on Linux,msvc(default on Windows). The defaults reproduce the previous build behavior; a non-default toolchain (or an externally-supplied compiler) is honored.Motivation: experiments (see the
build-wheel-riscv64-clangbranch in the private repo) showed >3x compile+link speedup with clang/lld vs gcc on Linux. This PR lands the build-backend support; a follow-up makes Linux CI toolchain-agnostic sollvmcan be selected there.Naming
gnu/llvm/msvcname toolchain families (compiler + linker + binutils), not just a compiler, which matches switching compiler and linker together.llvm(clang + lld) is preferred overclangbecause the linker also switches to lld;clangwould under-describe that. Values are case-insensitive; an invalid value or platform mismatch raises a clear error listing the platform's valid values.Resolution rules
_resolve_toolchain_name(shared by both packages) picks the toolchain as follows:CUDA_PYTHON_TOOLCHAINset — it takes precedence. The compiler/linker are set to the toolchain's (clang/clang+++-fuse-ld=lldforllvm). If an externally-setCCconflicts with it, a warning is emitted (CC only;CXXcommonly defaults toc++and is not a reliable user-intent signal) and the externalCCis overridden.CUDA_PYTHON_TOOLCHAINunset,CC/CXXset (Linux) — the toolchain is inferred from the external compiler (CXXpreferred, fall back toCC): a value containingclangselectsllvm, elsegnu. The inferred path uses that toolchain's flags but does not overrideCC/CXX, so a wrapper likeCC=sccache clangsurvives and gets the llvm flags. Inferredllvmruns the preflight so a missinglldfails fast.gnu/msvc), which does not touchCC/CXX(so the existingCC=sccache ccin CI keeps working).Preflight
_check_toolchain_availableprobes the toolchain's tools onPATHand fails fast with a helpful message (tool name, install hint, and how to fall back) instead of a cryptic compile error. No-op for the platform default; forllvmit checksclang,clang++, andld.lld.Changes
cuda_bindings/build_hooks.py,cuda_core/build_hooks.py: add the toolchain helpers. The genuinely-shared logic (constants, name resolution/validation, env application, preflight) is in a single block delimited by# --- begin/end shared toolchain helpers ---markers, duplicated verbatim with a "keep in sync" comment — mirroring the existing_import_get_cuda_path_or_homeprecedent. PEP 517 build isolation forbids a shared module (the sibling package isn't installed in the isolated build env), so the block is duplicated rather than imported; a test (test_shared_toolchain_block_is_in_sync) enforces the byte-identical invariant. The per-package_resolve_toolchain()is a thin wrapper that calls the shared helpers and assembles only its own package-specific flags (cuda.bindings: c++14,-fpermissive,-O3; cuda.core: c++17,-O2).cuda_bindings/setup.py: drops the now-redundant_is_clangstrip (the inference replaces it).cuda_core/setup.pyis untouched.CUDA_PYTHON_TOOLCHAINdocumented in both packages'environment_variables.rst.test_build_hooks.py; a module-level autouse fixture cleansCC/CXX/LDSHAREDper test (needed because_apply_toolchain_envwrites them directly toos.environ, whichmonkeypatchdoes not revert).Build paths verified
All current build entry points keep working: cibuildwheel (
build-wheel.yml, LinuxCC=sccache cc, Windows MSVC),python -m build/pip wheel(test-sdist-linux.yml,test-sdist-windows.yml),coverage.yml, pixipixi-build-python, the Cython-testbuild_tests.pydrivers, and localrebuild-cuda-python. The default path (neitherCUDA_PYTHON_TOOLCHAINnorCC/CXXset) is unchanged. An externally-suppliedCC=clangis now handled by inference (gets the llvm flag set) instead of the oldbuild_ext-time strip.Checklist