From 9c8377e6c6a06f47b750ba16bd1cd630bb0fc55f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 12:48:40 +0000 Subject: [PATCH 1/4] Build and test on Windows with MinGW GCC; drop clang from Linux CI CI's toolchains are upstream BART's: GCC 14 on Linux, Apple's clang on macOS, and MSYS2's UCRT64 GCC on Windows. The library builds there as a self-contained DLL: BART's win/ port is compiled in, what MinGW lacks (getsubopt, readlink, C11 threads) is supplied beside it, assertions reach BART's error path through the C runtime's import slots, and the GCC runtime, OpenMP and winpthreads are linked statically. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GEk7xWNYS2EqnCP1UMyzTg --- .github/workflows/copilot-setup-steps.yml | 6 +- .github/workflows/docs.yml | 11 +-- .github/workflows/tests.yml | 68 ++++++++++++++---- CMakeLists.txt | 88 +++++++++++++++++++---- scripts/gen_abi.py | 2 +- src/bartorch/_backend.py | 13 +++- src/bartorch/_finufft.py | 2 +- src/bartorch/_lib.py | 4 +- src/csrc/abi/api.c | 28 +++++++- src/csrc/abi/cuda.c | 5 ++ src/csrc/compat/c11threads/threads.h | 64 +++++++++++++++++ src/csrc/include/bartorch.h | 4 ++ src/csrc/substitute/posix.c | 56 +++++++++++++++ src/csrc/substitute/posix.h | 15 ++++ 14 files changed, 324 insertions(+), 42 deletions(-) create mode 100644 src/csrc/compat/c11threads/threads.h create mode 100644 src/csrc/substitute/posix.c create mode 100644 src/csrc/substitute/posix.h diff --git a/.github/workflows/copilot-setup-steps.yml b/.github/workflows/copilot-setup-steps.yml index 2dd40ce3..2e930544 100644 --- a/.github/workflows/copilot-setup-steps.yml +++ b/.github/workflows/copilot-setup-steps.yml @@ -20,7 +20,7 @@ jobs: submodules: recursive - name: Install compiler and CMake - run: sudo apt-get install -y cmake clang + run: sudo apt-get install -y cmake gcc-14 g++-14 - name: Install torch (CPU) and the build backend run: | @@ -37,6 +37,6 @@ jobs: - name: Build and install bartorch env: - CC: clang - CXX: clang++ + CC: gcc-14 + CXX: g++-14 run: pip install -e . --no-build-isolation diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 9c5b1197..89d987ca 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -107,9 +107,10 @@ jobs: python-version: "3.12" cache: pip - # libbartorch is a clang build: BART's nested functions become Blocks. - - name: Install the compiler, OpenMP and CMake - run: sudo apt-get install -y cmake clang libomp-dev + # GCC 14, as upstream BART builds on Linux: its heap trampolines are + # what BART's nested functions need. + - name: Install the compiler and CMake + run: sudo apt-get install -y cmake gcc-14 g++-14 - name: Install torch (CPU) and the build backend run: | @@ -119,8 +120,8 @@ jobs: - name: Build and install bartorch env: - CC: clang - CXX: clang++ + CC: gcc-14 + CXX: g++-14 run: pip install -e . --no-build-isolation - name: Install the documentation and example requirements diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bbef760a..df7fcec3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -10,7 +10,9 @@ env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" jobs: - # libbartorch is a clang build: BART's nested functions become Blocks. + # The toolchains are upstream BART's: GCC on Linux and on Windows, where + # it is MSYS2's MinGW build, and Apple's clang on macOS, where BART's nested + # functions become Blocks. tests: strategy: fail-fast: false @@ -18,25 +20,38 @@ jobs: include: - os: ubuntu-latest python-version: "3.10" - cc: clang + cc: gcc-14 + cxx: g++-14 - os: ubuntu-latest python-version: "3.12" - cc: clang + cc: gcc-14 + cxx: g++-14 # One job measures coverage for Codecov; the rest only test. coverage: true - os: ubuntu-latest python-version: "3.12" cc: gcc-14 + cxx: g++-14 extras: "[mkl]" - os: macos-latest python-version: "3.12" cc: clang + cxx: clang++ + - os: windows-latest + python-version: "3.12" + cc: gcc + cxx: g++ runs-on: ${{ matrix.os }} # Codecov takes the upload on GitHub's OIDC token, so no secret is needed. permissions: contents: read id-token: write + # Git's bash on Windows, so that every step reads the same everywhere. + defaults: + run: + shell: bash + steps: - uses: actions/checkout@v4 with: @@ -49,9 +64,29 @@ jobs: python-version: ${{ matrix.python-version }} cache: pip - - name: Install compiler, OpenMP and CMake (Linux) + - name: Install compiler and CMake (Linux) if: runner.os == 'Linux' - run: sudo apt-get install -y cmake ${{ matrix.cc }} libomp-dev + run: sudo apt-get install -y cmake ${{ matrix.cc }} ${{ matrix.cxx }} + + # MSYS2's UCRT64 GCC, which is how BART builds on Windows. The library + # links its GCC runtime, OpenMP and winpthreads statically, so nothing + # from MSYS2 has to be found when Python loads it; only the build uses + # this path. The interpreter is python.org's, as a user's would be. + - name: Install compiler and CMake (Windows) + if: runner.os == 'Windows' + id: msys2 + uses: msys2/setup-msys2@v2 + with: + msystem: UCRT64 + update: false + install: >- + mingw-w64-ucrt-x86_64-gcc + mingw-w64-ucrt-x86_64-cmake + mingw-w64-ucrt-x86_64-ninja + + - name: Put MSYS2's compiler on the path (Windows) + if: runner.os == 'Windows' + run: echo "$(cygpath -u '${{ steps.msys2.outputs.msys2-location }}')/ucrt64/bin" >> "$GITHUB_PATH" - name: Install CMake (macOS) if: runner.os == 'macOS' @@ -70,17 +105,21 @@ jobs: # # Every job needs this now: deepinv is a dependency rather than an # extra, so every install pulls torchvision whatever the matrix says. - # Linux only because that index publishes no macOS wheel -- there both + # Not on macOS, because that index publishes no macOS wheel -- there both # come from PyPI, which is consistent with itself. - name: Install torchvision from the same index as torch - if: runner.os == 'Linux' + if: runner.os != 'macOS' run: pip install torchvision --index-url https://download.pytorch.org/whl/cpu + # scikit-build-core asks for Visual Studio on Windows unless told + # otherwise, and MSVC cannot compile GNU C. - name: Build and install bartorch env: CC: ${{ matrix.cc }} - CXX: ${{ matrix.cc == 'gcc-14' && 'g++-14' || 'clang++' }} - run: pip install -e ".${{ matrix.extras }}" --no-build-isolation -v + CXX: ${{ matrix.cxx }} + run: | + if [ "$RUNNER_OS" = Windows ]; then export CMAKE_GENERATOR=Ninja; fi + pip install -e ".${{ matrix.extras }}" --no-build-isolation -v # Both wheels carry their own copy of LLVM's OpenMP runtime, and the # runtime ends the process rather than run beside a second copy. The @@ -154,8 +193,8 @@ jobs: python-version: "3.12" cache: pip - - name: Install the CUDA toolkit, clang and OpenMP - run: sudo apt-get update -qq && sudo apt-get install -y --no-install-recommends cmake clang libomp-dev nvidia-cuda-toolkit + - name: Install the CUDA toolkit and GCC + run: sudo apt-get update -qq && sudo apt-get install -y --no-install-recommends cmake gcc-14 g++-14 g++-12 nvidia-cuda-toolkit - name: Install torch (CPU) and the build backend run: | @@ -170,10 +209,13 @@ jobs: - name: Install torchvision from the same index as torch run: pip install torchvision --index-url https://download.pytorch.org/whl/cpu + # BART is GCC 14's, for its heap trampolines; the host side of the + # kernels is GCC 12's, the newest this distribution's nvcc accepts. - name: Build with BART's CUDA kernels env: - CC: clang - CXX: clang++ + CC: gcc-14 + CXX: g++-14 + CUDAHOSTCXX: g++-12 run: pip install -e . --no-build-isolation --config-settings=cmake.define.BARTORCH_CUDA=ON - name: The CUDA build must report its device code and run on the host diff --git a/CMakeLists.txt b/CMakeLists.txt index e1cd74c8..95fc2bd8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -88,16 +88,24 @@ elseif(CMAKE_C_COMPILER_ID STREQUAL "GNU") # GCC turns BART's nested functions into trampolines. On the stack those # need an executable stack, which glibc 2.41 refuses to dlopen; GCC 14 # places them on the heap instead, so the library loads anywhere. - if(CMAKE_C_COMPILER_VERSION VERSION_LESS 14) - message(FATAL_ERROR - "GCC ${CMAKE_C_COMPILER_VERSION} would need an executable stack for BART's nested " - "functions. Use GCC 14 or newer, which has -ftrampoline-impl=heap, or clang.") - endif() - list(APPEND BART_C_OPTIONS -ftrampoline-impl=heap + list(APPEND BART_C_OPTIONS -Wno-vla-parameter -Wno-nonnull -Wno-maybe-uninitialized -include assert.h) - list(APPEND BART_LINK_OPTIONS -Wl,-z,noexecstack) - set(BARTORCH_NESTED "gcc-heap-trampolines") + if(WIN32) + # MinGW's GCC makes the page of the stack a trampoline is written to + # executable itself (libgcc's __enable_execute_stack, VirtualProtect), + # which is how BART's own MSYS2 build runs its nested functions. + set(BARTORCH_NESTED "gcc-stack-trampolines") + else() + if(CMAKE_C_COMPILER_VERSION VERSION_LESS 14) + message(FATAL_ERROR + "GCC ${CMAKE_C_COMPILER_VERSION} would need an executable stack for BART's nested " + "functions. Use GCC 14 or newer, which has -ftrampoline-impl=heap, or clang.") + endif() + list(APPEND BART_C_OPTIONS -ftrampoline-impl=heap) + list(APPEND BART_LINK_OPTIONS -Wl,-z,noexecstack) + set(BARTORCH_NESTED "gcc-heap-trampolines") + endif() else() message(FATAL_ERROR "BART needs clang or GCC 14+; ${CMAKE_C_COMPILER_ID} cannot compile it") endif() @@ -125,7 +133,15 @@ endif() # would carry the toolchain's floor into every target system -- a GCC 14 build # asks for GCC_14.0.0 from libgcc_s, which no released distribution has -- and # that is a load-time failure, not a fallback. Static leaves libc and libgomp. -if(NOT APPLE) +# +# On Windows the same goes for everything MinGW would otherwise bring as a DLL +# -- libgomp, winpthreads, libgcc and libstdc++ -- because Python does not +# search PATH for a library's dependencies, so a DLL beside MSYS2's compiler is +# a DLL the interpreter cannot find. What is left is the C runtime and the +# system's own libraries. +if(WIN32) + list(APPEND BART_LINK_OPTIONS -static) +elseif(NOT APPLE) list(APPEND BART_LINK_OPTIONS -static-libgcc -static-libstdc++) endif() @@ -166,7 +182,12 @@ endif() if(BARTORCH_OPENMP) find_package(OpenMP COMPONENTS C) - if(OpenMP_C_FOUND) + if(OpenMP_C_FOUND AND WIN32) + # FindOpenMP names libgomp's import library, which -static cannot + # turn back into the archive; the flag lets the driver choose. + list(APPEND BART_C_OPTIONS ${OpenMP_C_FLAGS}) + list(APPEND BART_LINK_OPTIONS ${OpenMP_C_FLAGS}) + elseif(OpenMP_C_FOUND) list(APPEND BART_EXTRA_LIBS OpenMP::OpenMP_C) endif() endif() @@ -182,6 +203,12 @@ file(READ "${BART_ROOT}/version.txt" BART_VERSION) string(STRIP "${BART_VERSION}" BART_VERSION) file(WRITE "${BART_GEN}/misc/version.inc" "\"${BART_VERSION}\"\n") +# `tee` installs a SIGPIPE handler through sigaction, which Windows has +# neither of; it copies a stream between files, which the package does not +# expose anywhere (`_coverage.py`). +set(BART_TOOLS_NOT_ON_WINDOWS tee) +set(BART_SRC_NOT_ON_WINDOWS "${BART_SRC}/tee.c") + file(STRINGS "${BART_ROOT}/Makefile" _tool_lines REGEX "^T(BASE|FLP|NUM|IO|RECO|CALIB|MRI|SIM|NN|MOTION)\\+=") set(_mainlist "") set(_all_tools "") @@ -191,7 +218,7 @@ foreach(_cat BASE FLP NUM IO RECO CALIB MRI SIM NN MOTION) if(_line MATCHES "^T${_cat}\\+=(.*)$") string(REPLACE " " ";" _names "${CMAKE_MATCH_1}") foreach(_name ${_names}) - if(EXISTS "${BART_SRC}/${_name}.c") + if(EXISTS "${BART_SRC}/${_name}.c" AND NOT (WIN32 AND _name IN_LIST BART_TOOLS_NOT_ON_WINDOWS)) list(APPEND _tools "${_name}") list(APPEND _all_tools "${_name}") endif() @@ -230,7 +257,12 @@ file(GLOB _module_dirs LIST_DIRECTORIES true CONFIGURE_DEPENDS "${BART_SRC}/*") foreach(_dir ${_module_dirs}) if(IS_DIRECTORY "${_dir}") get_filename_component(_mod "${_dir}" NAME) - if(NOT _mod MATCHES "^(ismrm|lapacke|win)$") + # `win/` is BART's own port of mmap, fmemopen and the rest of what + # MinGW lacks, which its MSYS2 build links on Windows and nowhere else. + if(_mod STREQUAL "win" AND NOT WIN32) + continue() + endif() + if(NOT _mod MATCHES "^(ismrm|lapacke)$") file(GLOB _srcs CONFIGURE_DEPENDS "${_dir}/*.c") list(APPEND BART_SOURCES ${_srcs}) endif() @@ -250,6 +282,9 @@ list(REMOVE_ITEM BART_SOURCES "${BART_SRC}/bbox.c" "${BART_SRC}/ismrmrd.c" "${BART_SRC}/misc/memcfl.c") +if(WIN32) + list(REMOVE_ITEM BART_SOURCES ${BART_SRC_NOT_ON_WINDOWS}) +endif() # Three things, and a file belongs to whichever it is. `abi/` is the boundary: # what the host calls, and what BART's environment asks of the host in return. @@ -276,6 +311,10 @@ set(BARTORCH_SOURCES src/csrc/substitute/nufft_finufft.c src/csrc/substitute/psf.c) +if(WIN32) + list(APPEND BARTORCH_SOURCES src/csrc/substitute/posix.c) +endif() + if(BARTORCH_CUDA) list(APPEND BARTORCH_SOURCES src/csrc/ops/kernels.cu src/csrc/ops/fft_callbacks.cu) @@ -392,6 +431,31 @@ set_source_files_properties("${BART_SRC}/misc/mri2.c" PROPERTIES set_source_files_properties("${BART_SRC}/sense/optcom.c" PROPERTIES COMPILE_DEFINITIONS "estimate_scaling_norm=bart_estimate_scaling_norm") +# Windows, as BART's own MinGW build meets it. BARTLIB_STATIC keeps the +# handful of declarations BART marks for its bart.dll from asking for a +# dllimport of what is defined here. vptr.c installs a SIGSEGV handler through +# sigaction, which Windows lacks, unless it is built as that DLL is. bart.c +# raises SIGSTOP for its --attach debugging flag, which the package never +# passes and Windows has no signal for. misc/lock.c wants C11's , +# which MinGW's C library does not carry everywhere. src/csrc/substitute/posix.c +# is the rest of POSIX BART calls, declared into every file. NO_FIFO is what +# BART's own DLL is built with: a named pipe is a POSIX file. +if(WIN32) + target_compile_definitions(bartorch PRIVATE BARTLIB_STATIC NO_FIFO) + target_compile_options(bartorch PRIVATE + "$<$:SHELL:-include ${CMAKE_CURRENT_SOURCE_DIR}/src/csrc/substitute/posix.h>") + set_source_files_properties("${BART_SRC}/num/vptr.c" PROPERTIES + COMPILE_DEFINITIONS "BARTLIB_EXPORTS") + set_source_files_properties("${BART_SRC}/bart.c" PROPERTIES + COMPILE_DEFINITIONS "SIGSTOP=SIGABRT") + include(CheckIncludeFile) + check_include_file(threads.h BARTORCH_HAS_THREADS_H) + if(NOT BARTORCH_HAS_THREADS_H) + target_include_directories(bartorch PRIVATE + "${CMAKE_CURRENT_SOURCE_DIR}/src/csrc/compat/c11threads") + endif() +endif() + target_include_directories(bartorch PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}/src/csrc" "${CMAKE_CURRENT_SOURCE_DIR}/src/csrc/compat" diff --git a/scripts/gen_abi.py b/scripts/gen_abi.py index cd956180..e210dc09 100644 --- a/scripts/gen_abi.py +++ b/scripts/gen_abi.py @@ -71,7 +71,7 @@ def strip_comments(text: str) -> str: def strip_directives(text: str) -> str: - """Drop the preprocessor lines, including the two that define BARTORCH_API.""" + """Drop the preprocessor lines, including those that define BARTORCH_API.""" return "\n".join(line for line in text.splitlines() if not line.lstrip().startswith("#")) diff --git a/src/bartorch/_backend.py b/src/bartorch/_backend.py index 640a15eb..8396b8fc 100644 --- a/src/bartorch/_backend.py +++ b/src/bartorch/_backend.py @@ -84,9 +84,11 @@ def _mkl_library() -> Path | None: "libmkl_rt.so.2", "libmkl_rt.so.3", "libmkl_rt.so", + "mkl_rt.2.dll", + "mkl_rt.3.dll", ) for root in sorted(roots): - for sub in ("lib", "lib64"): + for sub in ("lib", "lib64", "Library/bin"): for name in names: candidate = root / sub / name if candidate.exists(): @@ -100,7 +102,9 @@ def _torch_library() -> Path | None: except ImportError: return None libdir = Path(torch.__file__).resolve().parent / "lib" - names = ["libtorch_cpu.dylib"] if sys.platform == "darwin" else ["libtorch_cpu.so"] + names = {"darwin": ["libtorch_cpu.dylib"], "win32": ["torch_cpu.dll"]}.get( + sys.platform, ["libtorch_cpu.so"] + ) for name in names: if (libdir / name).exists(): return libdir / name @@ -128,7 +132,10 @@ def _torch_providers() -> list[_Provider | None]: found.append( _open("Accelerate", "/System/Library/Frameworks/Accelerate.framework/Accelerate") ) - found.append(_open("process", None)) + # Windows has no handle on the process's own symbols: each DLL exports + # its own, and torch_cpu.dll is asked above. + if sys.platform != "win32": + found.append(_open("process", None)) return found diff --git a/src/bartorch/_finufft.py b/src/bartorch/_finufft.py index 6ffa491f..2743b322 100644 --- a/src/bartorch/_finufft.py +++ b/src/bartorch/_finufft.py @@ -68,7 +68,7 @@ def _library_path(package: str, stem: str) -> str | None: except ImportError: return None here = Path(module.__file__).resolve().parent - for name in (f"lib{stem}.so", f"lib{stem}.dylib"): + for name in (f"lib{stem}.so", f"lib{stem}.dylib", f"lib{stem}.dll"): candidate = here / name if candidate.exists(): return str(candidate) diff --git a/src/bartorch/_lib.py b/src/bartorch/_lib.py index 0cf661bf..054a909e 100644 --- a/src/bartorch/_lib.py +++ b/src/bartorch/_lib.py @@ -27,10 +27,10 @@ def _library_names() -> list[str]: - # No Windows name: BART does not build there, so neither does this, and - # WSL2 is a Linux install like any other. if sys.platform == "darwin": return ["libbartorch.dylib"] + if sys.platform == "win32": + return ["libbartorch.dll"] return ["libbartorch.so"] diff --git a/src/csrc/abi/api.c b/src/csrc/abi/api.c index 39dbf317..b03103d4 100644 --- a/src/csrc/abi/api.c +++ b/src/csrc/abi/api.c @@ -35,9 +35,33 @@ extern int bart_command(int len, char* buf, int argc, char* argv[]); * Apple's libc calls __assert_rtn(func, file, line, expr) -- a different name, * a different order, and a different type for the line. Answering only the * first left every one of BART's assertions aborting on macOS, which is a - * process death where every other platform gets an exception. + * process death where every other platform gets an exception. MinGW's + * assert() calls the C runtime's _assert, or _wassert under UNICODE; a DLL + * exports only what it marks for export, so these stay inside this library + * too. */ -#ifdef __APPLE__ +#if defined(_WIN32) + +#include + +__attribute__((noreturn)) +static void crt_assert(const char* assertion, const char* file, unsigned line) +{ + error("Assertion '%s' failed in %s:%u\n", assertion, file, line); +} + +__attribute__((noreturn)) +static void crt_wassert(const wchar_t* assertion, const wchar_t* file, unsigned line) +{ + error("Assertion '%ls' failed in %ls:%u\n", assertion, file, line); +} + +/* declares both dllimport, so a caller reaches them through these + * import-table slots; defined here, the import library's are never linked. */ +void (*__imp__assert)(const char*, const char*, unsigned) = crt_assert; +void (*__imp__wassert)(const wchar_t*, const wchar_t*, unsigned) = crt_wassert; + +#elif defined(__APPLE__) __attribute__((noreturn)) void __assert_rtn(const char* function, const char* file, int line, const char* assertion); diff --git a/src/csrc/abi/cuda.c b/src/csrc/abi/cuda.c index 4d0caf60..2a12535b 100644 --- a/src/csrc/abi/cuda.c +++ b/src/csrc/abi/cuda.c @@ -40,7 +40,12 @@ struct prefault { static void* prefault_run(void* arg) { struct prefault* p = arg; +#ifdef _WIN32 + /* The page size of x86-64 Windows; a smaller stride would still reach every page. */ + long page = 4096; +#else long page = sysconf(_SC_PAGESIZE); +#endif for (long i = 0; i < p->size; i += page) p->ptr[i] = p->ptr[i]; diff --git a/src/csrc/compat/c11threads/threads.h b/src/csrc/compat/c11threads/threads.h new file mode 100644 index 00000000..ff53ebf8 --- /dev/null +++ b/src/csrc/compat/c11threads/threads.h @@ -0,0 +1,64 @@ +/* + * The part of C11's BART's misc/lock.c uses, over pthreads. + * + * On the include path only where the C library has no of its own, + * which is MinGW's before winpthreads grew one; winpthreads is what serves it. + */ +#ifndef BARTORCH_C11THREADS_H +#define BARTORCH_C11THREADS_H + +#include + +typedef pthread_mutex_t mtx_t; +typedef pthread_cond_t cnd_t; + +enum { mtx_plain = 0 }; +enum { thrd_success = 0, thrd_error = 1, thrd_busy = 2 }; + +static inline int mtx_init(mtx_t* mx, int type) +{ + (void)type; + return pthread_mutex_init(mx, NULL) ? thrd_error : thrd_success; +} + +static inline int mtx_lock(mtx_t* mx) +{ + return pthread_mutex_lock(mx) ? thrd_error : thrd_success; +} + +static inline int mtx_trylock(mtx_t* mx) +{ + return pthread_mutex_trylock(mx) ? thrd_busy : thrd_success; +} + +static inline int mtx_unlock(mtx_t* mx) +{ + return pthread_mutex_unlock(mx) ? thrd_error : thrd_success; +} + +static inline void mtx_destroy(mtx_t* mx) +{ + pthread_mutex_destroy(mx); +} + +static inline int cnd_init(cnd_t* cnd) +{ + return pthread_cond_init(cnd, NULL) ? thrd_error : thrd_success; +} + +static inline int cnd_wait(cnd_t* cnd, mtx_t* mx) +{ + return pthread_cond_wait(cnd, mx) ? thrd_error : thrd_success; +} + +static inline int cnd_broadcast(cnd_t* cnd) +{ + return pthread_cond_broadcast(cnd) ? thrd_error : thrd_success; +} + +static inline void cnd_destroy(cnd_t* cnd) +{ + pthread_cond_destroy(cnd); +} + +#endif diff --git a/src/csrc/include/bartorch.h b/src/csrc/include/bartorch.h index e61b80bf..91ba5adb 100644 --- a/src/csrc/include/bartorch.h +++ b/src/csrc/include/bartorch.h @@ -18,7 +18,11 @@ extern "C" { #endif +#ifdef _WIN32 +#define BARTORCH_API __declspec(dllexport) +#else #define BARTORCH_API __attribute__((visibility("default"))) +#endif /* Number of dimensions BART carries for every array. */ #define BARTORCH_DIMS 16 diff --git a/src/csrc/substitute/posix.c b/src/csrc/substitute/posix.c new file mode 100644 index 00000000..679b2dc2 --- /dev/null +++ b/src/csrc/substitute/posix.c @@ -0,0 +1,56 @@ +/* + * The POSIX functions BART calls that MinGW's C library does not provide. + * + * Compiled on Windows only. getsubopt is POSIX's, since BART parses the + * sub-options of flags such as --nufft-conf with it. readlink is asked only + * for /proc/self/exe, to find a directory of external commands beside the + * `bart` executable; there is no such file on Windows and no such executable + * here, and the failure is what BART already expects on a system without one. + */ +#include +#include + +#include "substitute/posix.h" + +int getsubopt(char** optionp, char* const* tokens, char** valuep) +{ + char* option = *optionp; + + if ('\0' == *option) + return -1; + + char* end = strchr(option, ','); + + if (NULL == end) + end = option + strlen(option); + + char* equals = memchr(option, '=', (size_t)(end - option)); + size_t len = (size_t)((NULL != equals ? equals : end) - option); + + *optionp = ('\0' != *end) ? end + 1 : end; + + if ('\0' != *end) + *end = '\0'; + + for (int i = 0; NULL != tokens[i]; i++) { + + if ((0 == strncmp(option, tokens[i], len)) && ('\0' == tokens[i][len])) { + + *valuep = (NULL != equals) ? equals + 1 : NULL; + return i; + } + } + + *valuep = option; + return -1; +} + +ssize_t readlink(const char* path, char* buf, size_t size) +{ + (void)path; + (void)buf; + (void)size; + + errno = ENOSYS; + return -1; +} diff --git a/src/csrc/substitute/posix.h b/src/csrc/substitute/posix.h new file mode 100644 index 00000000..bfb567e2 --- /dev/null +++ b/src/csrc/substitute/posix.h @@ -0,0 +1,15 @@ +/* + * Declarations of what src/csrc/substitute/posix.c provides, included into + * every translation unit on Windows: GCC 14 refuses a call to an undeclared + * function, and MinGW's headers declare neither. + */ +#ifndef BARTORCH_SUBSTITUTE_POSIX_H +#define BARTORCH_SUBSTITUTE_POSIX_H + +#include +#include + +extern int getsubopt(char** optionp, char* const* tokens, char** valuep); +extern ssize_t readlink(const char* path, char* buf, size_t size); + +#endif From d06b9a22999c3eda9cd7130788aa445225da71f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 12:50:24 +0000 Subject: [PATCH 2/4] Fault prefault pages in through a helper rather than a self-assignment Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GEk7xWNYS2EqnCP1UMyzTg --- src/csrc/abi/cuda.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/csrc/abi/cuda.c b/src/csrc/abi/cuda.c index 2a12535b..cf3e1ea1 100644 --- a/src/csrc/abi/cuda.c +++ b/src/csrc/abi/cuda.c @@ -37,6 +37,13 @@ struct prefault { long size; }; +/* Writes back the byte it reads, which faults its page in writable. */ +static void touch(volatile char* at) +{ + char value = *at; + *at = value; +} + static void* prefault_run(void* arg) { struct prefault* p = arg; @@ -48,9 +55,9 @@ static void* prefault_run(void* arg) #endif for (long i = 0; i < p->size; i += page) - p->ptr[i] = p->ptr[i]; + touch(p->ptr + i); - p->ptr[p->size - 1] = p->ptr[p->size - 1]; + touch(p->ptr + p->size - 1); return NULL; } From 1a979cb193e41616381e6a39f884d64edf0fc7c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 12:52:02 +0000 Subject: [PATCH 3/4] Answer bart.c's readlink under a name of its own Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GEk7xWNYS2EqnCP1UMyzTg --- CMakeLists.txt | 5 +++-- src/csrc/substitute/posix.c | 11 ++++++----- src/csrc/substitute/posix.h | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 95fc2bd8..7d238dba 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -436,7 +436,8 @@ set_source_files_properties("${BART_SRC}/sense/optcom.c" PROPERTIES # dllimport of what is defined here. vptr.c installs a SIGSEGV handler through # sigaction, which Windows lacks, unless it is built as that DLL is. bart.c # raises SIGSTOP for its --attach debugging flag, which the package never -# passes and Windows has no signal for. misc/lock.c wants C11's , +# passes and Windows has no signal for, and its readlink of /proc/self/exe is +# answered by src/csrc/substitute/posix.c. misc/lock.c wants C11's , # which MinGW's C library does not carry everywhere. src/csrc/substitute/posix.c # is the rest of POSIX BART calls, declared into every file. NO_FIFO is what # BART's own DLL is built with: a named pipe is a POSIX file. @@ -447,7 +448,7 @@ if(WIN32) set_source_files_properties("${BART_SRC}/num/vptr.c" PROPERTIES COMPILE_DEFINITIONS "BARTLIB_EXPORTS") set_source_files_properties("${BART_SRC}/bart.c" PROPERTIES - COMPILE_DEFINITIONS "SIGSTOP=SIGABRT") + COMPILE_DEFINITIONS "SIGSTOP=SIGABRT;readlink=bartorch_no_readlink") include(CheckIncludeFile) check_include_file(threads.h BARTORCH_HAS_THREADS_H) if(NOT BARTORCH_HAS_THREADS_H) diff --git a/src/csrc/substitute/posix.c b/src/csrc/substitute/posix.c index 679b2dc2..6ea86a1f 100644 --- a/src/csrc/substitute/posix.c +++ b/src/csrc/substitute/posix.c @@ -2,10 +2,11 @@ * The POSIX functions BART calls that MinGW's C library does not provide. * * Compiled on Windows only. getsubopt is POSIX's, since BART parses the - * sub-options of flags such as --nufft-conf with it. readlink is asked only - * for /proc/self/exe, to find a directory of external commands beside the - * `bart` executable; there is no such file on Windows and no such executable - * here, and the failure is what BART already expects on a system without one. + * sub-options of flags such as --nufft-conf with it. bart.c's one readlink + * call, which CMake renames to bartorch_no_readlink, asks for /proc/self/exe + * to find a directory of external commands beside the `bart` executable; there + * is no such file on Windows and no such executable here, and the failure is + * what BART already expects on a system without one. */ #include #include @@ -45,7 +46,7 @@ int getsubopt(char** optionp, char* const* tokens, char** valuep) return -1; } -ssize_t readlink(const char* path, char* buf, size_t size) +ssize_t bartorch_no_readlink(const char* path, char* buf, size_t size) { (void)path; (void)buf; diff --git a/src/csrc/substitute/posix.h b/src/csrc/substitute/posix.h index bfb567e2..fc9fe133 100644 --- a/src/csrc/substitute/posix.h +++ b/src/csrc/substitute/posix.h @@ -10,6 +10,6 @@ #include extern int getsubopt(char** optionp, char* const* tokens, char** valuep); -extern ssize_t readlink(const char* path, char* buf, size_t size); +extern ssize_t bartorch_no_readlink(const char* path, char* buf, size_t size); #endif From a8d99069a268c6000dd386b59668df95eb1820eb Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 13:16:29 +0000 Subject: [PATCH 4/4] Drop Windows again; test the oldest and newest Python per platform BART builds under MinGW, but Windows' 32-bit long caps every array at 2 GiB and no macro can widen it without editing BART, so the Windows build is reverted and the documentation says why. The test matrix runs 3.10 and 3.14 on Linux (GCC 14) and macOS (clang); the CUDA wheel is loaded from every supported Python, as the CPU wheels are tested. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GEk7xWNYS2EqnCP1UMyzTg --- .github/workflows/publish.yml | 18 ++++-- .github/workflows/tests.yml | 61 +++++------------- AGENTS.md | 18 ++++-- CMakeLists.txt | 89 ++++---------------------- docs/guides/developer/prerequisites.md | 3 +- docs/guides/developer/pull-requests.md | 3 +- docs/guides/user/prerequisites.md | 2 +- pyproject.toml | 2 +- scripts/gen_abi.py | 2 +- src/bartorch/_backend.py | 13 +--- src/bartorch/_finufft.py | 2 +- src/bartorch/_lib.py | 4 +- src/csrc/abi/api.c | 28 +------- src/csrc/abi/cuda.c | 16 +---- src/csrc/compat/c11threads/threads.h | 64 ------------------ src/csrc/include/bartorch.h | 4 -- src/csrc/substitute/posix.c | 57 ----------------- src/csrc/substitute/posix.h | 15 ----- 18 files changed, 67 insertions(+), 334 deletions(-) delete mode 100644 src/csrc/compat/c11threads/threads.h delete mode 100644 src/csrc/substitute/posix.c delete mode 100644 src/csrc/substitute/posix.h diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 8297ef88..cf59360f 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -182,19 +182,25 @@ jobs: -w wheelhouse/ --plat manylinux_2_28_x86_64 \ --exclude libcudart.so.12 --exclude libcufft.so.11 \ --exclude libcublas.so.12 --exclude libcublasLt.so.12 - - name: Check that it loads without a card + # The wheel carries no interpreter, so it is loaded from each Python the + # package supports, as the CPU wheels are tested from each. + - name: Check that it loads without a card, on every Python run: | - /opt/python/cp312-cp312/bin/python -m pip install wheelhouse/*.whl \ - torch --index-url https://download.pytorch.org/whl/cpu \ - --extra-index-url https://pypi.org/simple - /opt/python/cp312-cp312/bin/python -c " + for py in cp310-cp310 cp311-cp311 cp312-cp312 cp313-cp313 cp314-cp314; do + python=/opt/python/$py/bin/python + $python -m pip install wheelhouse/*.whl \ + torch --index-url https://download.pytorch.org/whl/cpu \ + --extra-index-url https://pypi.org/simple + $python -c " + import sys import bartorch from bartorch import _cuda info = bartorch.build_info() - print(info) + print(sys.version.split()[0], info) assert 'cuda=ON' in info, info print('devices:', _cuda.device_count()) " + done - uses: actions/upload-artifact@v4 with: name: cuda-wheel diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index df7fcec3..337f4ad8 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -10,9 +10,10 @@ env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" jobs: - # The toolchains are upstream BART's: GCC on Linux and on Windows, where - # it is MSYS2's MinGW build, and Apple's clang on macOS, where BART's nested - # functions become Blocks. + # The toolchains are upstream BART's: GCC 14 on Linux, whose heap + # trampolines carry BART's nested functions, and Apple's clang on macOS, + # where they become Blocks. Each platform runs the oldest and the newest + # Python the package supports; the wheel workflow covers every version. tests: strategy: fail-fast: false @@ -22,36 +23,28 @@ jobs: python-version: "3.10" cc: gcc-14 cxx: g++-14 + # MKL as the BLAS, LAPACK and FFT source, in one of the two jobs. + extras: "[mkl]" - os: ubuntu-latest - python-version: "3.12" + python-version: "3.14" cc: gcc-14 cxx: g++-14 # One job measures coverage for Codecov; the rest only test. coverage: true - - os: ubuntu-latest - python-version: "3.12" - cc: gcc-14 - cxx: g++-14 - extras: "[mkl]" - os: macos-latest - python-version: "3.12" + python-version: "3.10" + cc: clang + cxx: clang++ + - os: macos-latest + python-version: "3.14" cc: clang cxx: clang++ - - os: windows-latest - python-version: "3.12" - cc: gcc - cxx: g++ runs-on: ${{ matrix.os }} # Codecov takes the upload on GitHub's OIDC token, so no secret is needed. permissions: contents: read id-token: write - # Git's bash on Windows, so that every step reads the same everywhere. - defaults: - run: - shell: bash - steps: - uses: actions/checkout@v4 with: @@ -68,26 +61,6 @@ jobs: if: runner.os == 'Linux' run: sudo apt-get install -y cmake ${{ matrix.cc }} ${{ matrix.cxx }} - # MSYS2's UCRT64 GCC, which is how BART builds on Windows. The library - # links its GCC runtime, OpenMP and winpthreads statically, so nothing - # from MSYS2 has to be found when Python loads it; only the build uses - # this path. The interpreter is python.org's, as a user's would be. - - name: Install compiler and CMake (Windows) - if: runner.os == 'Windows' - id: msys2 - uses: msys2/setup-msys2@v2 - with: - msystem: UCRT64 - update: false - install: >- - mingw-w64-ucrt-x86_64-gcc - mingw-w64-ucrt-x86_64-cmake - mingw-w64-ucrt-x86_64-ninja - - - name: Put MSYS2's compiler on the path (Windows) - if: runner.os == 'Windows' - run: echo "$(cygpath -u '${{ steps.msys2.outputs.msys2-location }}')/ucrt64/bin" >> "$GITHUB_PATH" - - name: Install CMake (macOS) if: runner.os == 'macOS' run: brew install cmake @@ -105,21 +78,17 @@ jobs: # # Every job needs this now: deepinv is a dependency rather than an # extra, so every install pulls torchvision whatever the matrix says. - # Not on macOS, because that index publishes no macOS wheel -- there both + # Linux only because that index publishes no macOS wheel -- there both # come from PyPI, which is consistent with itself. - name: Install torchvision from the same index as torch - if: runner.os != 'macOS' + if: runner.os == 'Linux' run: pip install torchvision --index-url https://download.pytorch.org/whl/cpu - # scikit-build-core asks for Visual Studio on Windows unless told - # otherwise, and MSVC cannot compile GNU C. - name: Build and install bartorch env: CC: ${{ matrix.cc }} CXX: ${{ matrix.cxx }} - run: | - if [ "$RUNNER_OS" = Windows ]; then export CMAKE_GENERATOR=Ninja; fi - pip install -e ".${{ matrix.extras }}" --no-build-isolation -v + run: pip install -e ".${{ matrix.extras }}" --no-build-isolation -v # Both wheels carry their own copy of LLVM's OpenMP runtime, and the # runtime ends the process rather than run beside a second copy. The diff --git a/AGENTS.md b/AGENTS.md index 4c3e4a3e..beaa3985 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -614,12 +614,16 @@ the stack needs an executable stack, which glibc 2.41 refuses to `dlopen`; so GCC 14's `-ftrampoline-impl=heap` is required and older GCC is rejected at configure time. BART's own `NOEXEC_STACK` workaround does not help here: it parses a trampoline layout GCC emits only for non-PIC executables, not for a -shared library. Both compilers are built and tested in CI. +shared library. CI builds with upstream BART's toolchains: GCC 14 on Linux +and Apple's clang on macOS. clang on Linux, with the vendored Blocks runtime, +still builds and is not tested in CI. -Windows is not a platform here. BART does not build on it, and nothing in -this repository carries a path toward one: no `.dll` among the names the -loader tries, no `__declspec(dllexport)`, no `win32` branch picking a -different library. WSL2 is a Linux install and is the answer. +Windows is not a platform here. BART builds there with MinGW, but Windows is +LLP64: `long` is 32 bits, and BART holds every dimension, size and byte +stride in a `long`, so no array could exceed 2 GiB. No macro fixes that +without editing BART: `#define long long long` breaks every `long long` BART +writes, a typedef breaks every `unsigned long`, and its `sscanf("%lu")` calls +would still write four bytes. WSL2 is a Linux install and is the answer. The compiler's own runtime is linked statically on Linux, because otherwise the toolchain's floor becomes the target system's: a GCC 14 build asks @@ -1109,8 +1113,8 @@ of it is written. What comes back is finite stack memory. `_call`'s `_PRECONDITIONS` refuses those combinations before the command runs, which is where any further "BART does not define this" case belongs. -Windows is not on this list because it is not a target: BART does not build -there, and WSL2 is a Linux install like any other. +Windows is not on this list because it is not a target: BART's 32-bit `long` +there caps every array at 2 GiB, and WSL2 is a Linux install like any other. A tool that takes device memory as it stands. BART guards the host reads that would break -- `estimate_im_dims` copies to the host when it is handed one -- diff --git a/CMakeLists.txt b/CMakeLists.txt index 7d238dba..e1cd74c8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -88,24 +88,16 @@ elseif(CMAKE_C_COMPILER_ID STREQUAL "GNU") # GCC turns BART's nested functions into trampolines. On the stack those # need an executable stack, which glibc 2.41 refuses to dlopen; GCC 14 # places them on the heap instead, so the library loads anywhere. - list(APPEND BART_C_OPTIONS + if(CMAKE_C_COMPILER_VERSION VERSION_LESS 14) + message(FATAL_ERROR + "GCC ${CMAKE_C_COMPILER_VERSION} would need an executable stack for BART's nested " + "functions. Use GCC 14 or newer, which has -ftrampoline-impl=heap, or clang.") + endif() + list(APPEND BART_C_OPTIONS -ftrampoline-impl=heap -Wno-vla-parameter -Wno-nonnull -Wno-maybe-uninitialized -include assert.h) - if(WIN32) - # MinGW's GCC makes the page of the stack a trampoline is written to - # executable itself (libgcc's __enable_execute_stack, VirtualProtect), - # which is how BART's own MSYS2 build runs its nested functions. - set(BARTORCH_NESTED "gcc-stack-trampolines") - else() - if(CMAKE_C_COMPILER_VERSION VERSION_LESS 14) - message(FATAL_ERROR - "GCC ${CMAKE_C_COMPILER_VERSION} would need an executable stack for BART's nested " - "functions. Use GCC 14 or newer, which has -ftrampoline-impl=heap, or clang.") - endif() - list(APPEND BART_C_OPTIONS -ftrampoline-impl=heap) - list(APPEND BART_LINK_OPTIONS -Wl,-z,noexecstack) - set(BARTORCH_NESTED "gcc-heap-trampolines") - endif() + list(APPEND BART_LINK_OPTIONS -Wl,-z,noexecstack) + set(BARTORCH_NESTED "gcc-heap-trampolines") else() message(FATAL_ERROR "BART needs clang or GCC 14+; ${CMAKE_C_COMPILER_ID} cannot compile it") endif() @@ -133,15 +125,7 @@ endif() # would carry the toolchain's floor into every target system -- a GCC 14 build # asks for GCC_14.0.0 from libgcc_s, which no released distribution has -- and # that is a load-time failure, not a fallback. Static leaves libc and libgomp. -# -# On Windows the same goes for everything MinGW would otherwise bring as a DLL -# -- libgomp, winpthreads, libgcc and libstdc++ -- because Python does not -# search PATH for a library's dependencies, so a DLL beside MSYS2's compiler is -# a DLL the interpreter cannot find. What is left is the C runtime and the -# system's own libraries. -if(WIN32) - list(APPEND BART_LINK_OPTIONS -static) -elseif(NOT APPLE) +if(NOT APPLE) list(APPEND BART_LINK_OPTIONS -static-libgcc -static-libstdc++) endif() @@ -182,12 +166,7 @@ endif() if(BARTORCH_OPENMP) find_package(OpenMP COMPONENTS C) - if(OpenMP_C_FOUND AND WIN32) - # FindOpenMP names libgomp's import library, which -static cannot - # turn back into the archive; the flag lets the driver choose. - list(APPEND BART_C_OPTIONS ${OpenMP_C_FLAGS}) - list(APPEND BART_LINK_OPTIONS ${OpenMP_C_FLAGS}) - elseif(OpenMP_C_FOUND) + if(OpenMP_C_FOUND) list(APPEND BART_EXTRA_LIBS OpenMP::OpenMP_C) endif() endif() @@ -203,12 +182,6 @@ file(READ "${BART_ROOT}/version.txt" BART_VERSION) string(STRIP "${BART_VERSION}" BART_VERSION) file(WRITE "${BART_GEN}/misc/version.inc" "\"${BART_VERSION}\"\n") -# `tee` installs a SIGPIPE handler through sigaction, which Windows has -# neither of; it copies a stream between files, which the package does not -# expose anywhere (`_coverage.py`). -set(BART_TOOLS_NOT_ON_WINDOWS tee) -set(BART_SRC_NOT_ON_WINDOWS "${BART_SRC}/tee.c") - file(STRINGS "${BART_ROOT}/Makefile" _tool_lines REGEX "^T(BASE|FLP|NUM|IO|RECO|CALIB|MRI|SIM|NN|MOTION)\\+=") set(_mainlist "") set(_all_tools "") @@ -218,7 +191,7 @@ foreach(_cat BASE FLP NUM IO RECO CALIB MRI SIM NN MOTION) if(_line MATCHES "^T${_cat}\\+=(.*)$") string(REPLACE " " ";" _names "${CMAKE_MATCH_1}") foreach(_name ${_names}) - if(EXISTS "${BART_SRC}/${_name}.c" AND NOT (WIN32 AND _name IN_LIST BART_TOOLS_NOT_ON_WINDOWS)) + if(EXISTS "${BART_SRC}/${_name}.c") list(APPEND _tools "${_name}") list(APPEND _all_tools "${_name}") endif() @@ -257,12 +230,7 @@ file(GLOB _module_dirs LIST_DIRECTORIES true CONFIGURE_DEPENDS "${BART_SRC}/*") foreach(_dir ${_module_dirs}) if(IS_DIRECTORY "${_dir}") get_filename_component(_mod "${_dir}" NAME) - # `win/` is BART's own port of mmap, fmemopen and the rest of what - # MinGW lacks, which its MSYS2 build links on Windows and nowhere else. - if(_mod STREQUAL "win" AND NOT WIN32) - continue() - endif() - if(NOT _mod MATCHES "^(ismrm|lapacke)$") + if(NOT _mod MATCHES "^(ismrm|lapacke|win)$") file(GLOB _srcs CONFIGURE_DEPENDS "${_dir}/*.c") list(APPEND BART_SOURCES ${_srcs}) endif() @@ -282,9 +250,6 @@ list(REMOVE_ITEM BART_SOURCES "${BART_SRC}/bbox.c" "${BART_SRC}/ismrmrd.c" "${BART_SRC}/misc/memcfl.c") -if(WIN32) - list(REMOVE_ITEM BART_SOURCES ${BART_SRC_NOT_ON_WINDOWS}) -endif() # Three things, and a file belongs to whichever it is. `abi/` is the boundary: # what the host calls, and what BART's environment asks of the host in return. @@ -311,10 +276,6 @@ set(BARTORCH_SOURCES src/csrc/substitute/nufft_finufft.c src/csrc/substitute/psf.c) -if(WIN32) - list(APPEND BARTORCH_SOURCES src/csrc/substitute/posix.c) -endif() - if(BARTORCH_CUDA) list(APPEND BARTORCH_SOURCES src/csrc/ops/kernels.cu src/csrc/ops/fft_callbacks.cu) @@ -431,32 +392,6 @@ set_source_files_properties("${BART_SRC}/misc/mri2.c" PROPERTIES set_source_files_properties("${BART_SRC}/sense/optcom.c" PROPERTIES COMPILE_DEFINITIONS "estimate_scaling_norm=bart_estimate_scaling_norm") -# Windows, as BART's own MinGW build meets it. BARTLIB_STATIC keeps the -# handful of declarations BART marks for its bart.dll from asking for a -# dllimport of what is defined here. vptr.c installs a SIGSEGV handler through -# sigaction, which Windows lacks, unless it is built as that DLL is. bart.c -# raises SIGSTOP for its --attach debugging flag, which the package never -# passes and Windows has no signal for, and its readlink of /proc/self/exe is -# answered by src/csrc/substitute/posix.c. misc/lock.c wants C11's , -# which MinGW's C library does not carry everywhere. src/csrc/substitute/posix.c -# is the rest of POSIX BART calls, declared into every file. NO_FIFO is what -# BART's own DLL is built with: a named pipe is a POSIX file. -if(WIN32) - target_compile_definitions(bartorch PRIVATE BARTLIB_STATIC NO_FIFO) - target_compile_options(bartorch PRIVATE - "$<$:SHELL:-include ${CMAKE_CURRENT_SOURCE_DIR}/src/csrc/substitute/posix.h>") - set_source_files_properties("${BART_SRC}/num/vptr.c" PROPERTIES - COMPILE_DEFINITIONS "BARTLIB_EXPORTS") - set_source_files_properties("${BART_SRC}/bart.c" PROPERTIES - COMPILE_DEFINITIONS "SIGSTOP=SIGABRT;readlink=bartorch_no_readlink") - include(CheckIncludeFile) - check_include_file(threads.h BARTORCH_HAS_THREADS_H) - if(NOT BARTORCH_HAS_THREADS_H) - target_include_directories(bartorch PRIVATE - "${CMAKE_CURRENT_SOURCE_DIR}/src/csrc/compat/c11threads") - endif() -endif() - target_include_directories(bartorch PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}/src/csrc" "${CMAKE_CURRENT_SOURCE_DIR}/src/csrc/compat" diff --git a/docs/guides/developer/prerequisites.md b/docs/guides/developer/prerequisites.md index 8980dd2f..243b8efa 100644 --- a/docs/guides/developer/prerequisites.md +++ b/docs/guides/developer/prerequisites.md @@ -15,5 +15,6 @@ suite is run from `src/` without installing, install it separately skip, because the substitution declining is an error. `pip install mkl deepinv` enables the tests that need MKL and the DeepInverse adapter. -Windows is not a target: BART does not build on it, and WSL2 is used as a Linux +Windows is not a target: BART stores array sizes and strides in `long`, which +is 32 bits there and limits every array to 2 GiB. WSL2 is used as a Linux environment. diff --git a/docs/guides/developer/pull-requests.md b/docs/guides/developer/pull-requests.md index 40edf072..5d00ed05 100644 --- a/docs/guides/developer/pull-requests.md +++ b/docs/guides/developer/pull-requests.md @@ -11,7 +11,8 @@ template: | Validation | The commands run and their results; for a numerical change, the reference and the tolerance; hardware or optional dependencies that were not available | | Public API and documentation | Changes to public behaviour and the documentation updated for them | -CI runs the test matrix (Linux with clang and GCC 14, macOS, and a CUDA build +CI runs the test matrix (Linux with GCC 14 and macOS with clang, each on the +oldest and newest supported Python, and a CUDA build without a device), the lint and spelling checks, and two documentation builds: the reference without the compiled library, and the executed gallery, which is published from `main` and from release tags. A pull request is merged when CI passes and a diff --git a/docs/guides/user/prerequisites.md b/docs/guides/user/prerequisites.md index c86ef41d..80cc6f80 100644 --- a/docs/guides/user/prerequisites.md +++ b/docs/guides/user/prerequisites.md @@ -18,7 +18,7 @@ | macOS 11 or later on Apple silicon | Wheel | CPU only; see {ref}`macos-openmp` | | Linux aarch64 | Source distribution | BART is compiled on installation; FINUFFT publishes no wheel for this platform and is built from source as well | | macOS on Intel | Source distribution | BART is compiled on installation; FINUFFT releases after 2.4.0 have no wheel for this platform and are built from source as well | -| Windows | Not supported | BART does not build on Windows; WSL2 provides a Linux environment | +| Windows | Not supported | BART stores array sizes and strides in `long`, which is 32 bits on Windows, limiting every array to 2 GiB; WSL2 provides a Linux environment | A source installation needs the toolchain listed under {ref}`source-builds`. Apple MPS devices are not supported; the device paths are CPU and CUDA. diff --git a/pyproject.toml b/pyproject.toml index 62c1eef3..b8860b3f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -80,7 +80,7 @@ deepinv = [ # MKL covers every BLAS and LAPACK routine BART calls and is about twice as # fast as the alternatives on the work that leans on LAPACK. There is no MKL # wheel for macOS, where SciPy's OpenBLAS and Accelerate serve instead, and -# Windows is not a target: BART does not build there. +# Windows is not a target: BART's `long` is 32 bits there. mkl = [ "mkl>=2024; sys_platform == 'linux' and platform_machine == 'x86_64'", ] diff --git a/scripts/gen_abi.py b/scripts/gen_abi.py index e210dc09..cd956180 100644 --- a/scripts/gen_abi.py +++ b/scripts/gen_abi.py @@ -71,7 +71,7 @@ def strip_comments(text: str) -> str: def strip_directives(text: str) -> str: - """Drop the preprocessor lines, including those that define BARTORCH_API.""" + """Drop the preprocessor lines, including the two that define BARTORCH_API.""" return "\n".join(line for line in text.splitlines() if not line.lstrip().startswith("#")) diff --git a/src/bartorch/_backend.py b/src/bartorch/_backend.py index 8396b8fc..640a15eb 100644 --- a/src/bartorch/_backend.py +++ b/src/bartorch/_backend.py @@ -84,11 +84,9 @@ def _mkl_library() -> Path | None: "libmkl_rt.so.2", "libmkl_rt.so.3", "libmkl_rt.so", - "mkl_rt.2.dll", - "mkl_rt.3.dll", ) for root in sorted(roots): - for sub in ("lib", "lib64", "Library/bin"): + for sub in ("lib", "lib64"): for name in names: candidate = root / sub / name if candidate.exists(): @@ -102,9 +100,7 @@ def _torch_library() -> Path | None: except ImportError: return None libdir = Path(torch.__file__).resolve().parent / "lib" - names = {"darwin": ["libtorch_cpu.dylib"], "win32": ["torch_cpu.dll"]}.get( - sys.platform, ["libtorch_cpu.so"] - ) + names = ["libtorch_cpu.dylib"] if sys.platform == "darwin" else ["libtorch_cpu.so"] for name in names: if (libdir / name).exists(): return libdir / name @@ -132,10 +128,7 @@ def _torch_providers() -> list[_Provider | None]: found.append( _open("Accelerate", "/System/Library/Frameworks/Accelerate.framework/Accelerate") ) - # Windows has no handle on the process's own symbols: each DLL exports - # its own, and torch_cpu.dll is asked above. - if sys.platform != "win32": - found.append(_open("process", None)) + found.append(_open("process", None)) return found diff --git a/src/bartorch/_finufft.py b/src/bartorch/_finufft.py index 2743b322..6ffa491f 100644 --- a/src/bartorch/_finufft.py +++ b/src/bartorch/_finufft.py @@ -68,7 +68,7 @@ def _library_path(package: str, stem: str) -> str | None: except ImportError: return None here = Path(module.__file__).resolve().parent - for name in (f"lib{stem}.so", f"lib{stem}.dylib", f"lib{stem}.dll"): + for name in (f"lib{stem}.so", f"lib{stem}.dylib"): candidate = here / name if candidate.exists(): return str(candidate) diff --git a/src/bartorch/_lib.py b/src/bartorch/_lib.py index 054a909e..0cf661bf 100644 --- a/src/bartorch/_lib.py +++ b/src/bartorch/_lib.py @@ -27,10 +27,10 @@ def _library_names() -> list[str]: + # No Windows name: BART does not build there, so neither does this, and + # WSL2 is a Linux install like any other. if sys.platform == "darwin": return ["libbartorch.dylib"] - if sys.platform == "win32": - return ["libbartorch.dll"] return ["libbartorch.so"] diff --git a/src/csrc/abi/api.c b/src/csrc/abi/api.c index b03103d4..39dbf317 100644 --- a/src/csrc/abi/api.c +++ b/src/csrc/abi/api.c @@ -35,33 +35,9 @@ extern int bart_command(int len, char* buf, int argc, char* argv[]); * Apple's libc calls __assert_rtn(func, file, line, expr) -- a different name, * a different order, and a different type for the line. Answering only the * first left every one of BART's assertions aborting on macOS, which is a - * process death where every other platform gets an exception. MinGW's - * assert() calls the C runtime's _assert, or _wassert under UNICODE; a DLL - * exports only what it marks for export, so these stay inside this library - * too. + * process death where every other platform gets an exception. */ -#if defined(_WIN32) - -#include - -__attribute__((noreturn)) -static void crt_assert(const char* assertion, const char* file, unsigned line) -{ - error("Assertion '%s' failed in %s:%u\n", assertion, file, line); -} - -__attribute__((noreturn)) -static void crt_wassert(const wchar_t* assertion, const wchar_t* file, unsigned line) -{ - error("Assertion '%ls' failed in %ls:%u\n", assertion, file, line); -} - -/* declares both dllimport, so a caller reaches them through these - * import-table slots; defined here, the import library's are never linked. */ -void (*__imp__assert)(const char*, const char*, unsigned) = crt_assert; -void (*__imp__wassert)(const wchar_t*, const wchar_t*, unsigned) = crt_wassert; - -#elif defined(__APPLE__) +#ifdef __APPLE__ __attribute__((noreturn)) void __assert_rtn(const char* function, const char* file, int line, const char* assertion); diff --git a/src/csrc/abi/cuda.c b/src/csrc/abi/cuda.c index cf3e1ea1..4d0caf60 100644 --- a/src/csrc/abi/cuda.c +++ b/src/csrc/abi/cuda.c @@ -37,27 +37,15 @@ struct prefault { long size; }; -/* Writes back the byte it reads, which faults its page in writable. */ -static void touch(volatile char* at) -{ - char value = *at; - *at = value; -} - static void* prefault_run(void* arg) { struct prefault* p = arg; -#ifdef _WIN32 - /* The page size of x86-64 Windows; a smaller stride would still reach every page. */ - long page = 4096; -#else long page = sysconf(_SC_PAGESIZE); -#endif for (long i = 0; i < p->size; i += page) - touch(p->ptr + i); + p->ptr[i] = p->ptr[i]; - touch(p->ptr + p->size - 1); + p->ptr[p->size - 1] = p->ptr[p->size - 1]; return NULL; } diff --git a/src/csrc/compat/c11threads/threads.h b/src/csrc/compat/c11threads/threads.h deleted file mode 100644 index ff53ebf8..00000000 --- a/src/csrc/compat/c11threads/threads.h +++ /dev/null @@ -1,64 +0,0 @@ -/* - * The part of C11's BART's misc/lock.c uses, over pthreads. - * - * On the include path only where the C library has no of its own, - * which is MinGW's before winpthreads grew one; winpthreads is what serves it. - */ -#ifndef BARTORCH_C11THREADS_H -#define BARTORCH_C11THREADS_H - -#include - -typedef pthread_mutex_t mtx_t; -typedef pthread_cond_t cnd_t; - -enum { mtx_plain = 0 }; -enum { thrd_success = 0, thrd_error = 1, thrd_busy = 2 }; - -static inline int mtx_init(mtx_t* mx, int type) -{ - (void)type; - return pthread_mutex_init(mx, NULL) ? thrd_error : thrd_success; -} - -static inline int mtx_lock(mtx_t* mx) -{ - return pthread_mutex_lock(mx) ? thrd_error : thrd_success; -} - -static inline int mtx_trylock(mtx_t* mx) -{ - return pthread_mutex_trylock(mx) ? thrd_busy : thrd_success; -} - -static inline int mtx_unlock(mtx_t* mx) -{ - return pthread_mutex_unlock(mx) ? thrd_error : thrd_success; -} - -static inline void mtx_destroy(mtx_t* mx) -{ - pthread_mutex_destroy(mx); -} - -static inline int cnd_init(cnd_t* cnd) -{ - return pthread_cond_init(cnd, NULL) ? thrd_error : thrd_success; -} - -static inline int cnd_wait(cnd_t* cnd, mtx_t* mx) -{ - return pthread_cond_wait(cnd, mx) ? thrd_error : thrd_success; -} - -static inline int cnd_broadcast(cnd_t* cnd) -{ - return pthread_cond_broadcast(cnd) ? thrd_error : thrd_success; -} - -static inline void cnd_destroy(cnd_t* cnd) -{ - pthread_cond_destroy(cnd); -} - -#endif diff --git a/src/csrc/include/bartorch.h b/src/csrc/include/bartorch.h index 91ba5adb..e61b80bf 100644 --- a/src/csrc/include/bartorch.h +++ b/src/csrc/include/bartorch.h @@ -18,11 +18,7 @@ extern "C" { #endif -#ifdef _WIN32 -#define BARTORCH_API __declspec(dllexport) -#else #define BARTORCH_API __attribute__((visibility("default"))) -#endif /* Number of dimensions BART carries for every array. */ #define BARTORCH_DIMS 16 diff --git a/src/csrc/substitute/posix.c b/src/csrc/substitute/posix.c deleted file mode 100644 index 6ea86a1f..00000000 --- a/src/csrc/substitute/posix.c +++ /dev/null @@ -1,57 +0,0 @@ -/* - * The POSIX functions BART calls that MinGW's C library does not provide. - * - * Compiled on Windows only. getsubopt is POSIX's, since BART parses the - * sub-options of flags such as --nufft-conf with it. bart.c's one readlink - * call, which CMake renames to bartorch_no_readlink, asks for /proc/self/exe - * to find a directory of external commands beside the `bart` executable; there - * is no such file on Windows and no such executable here, and the failure is - * what BART already expects on a system without one. - */ -#include -#include - -#include "substitute/posix.h" - -int getsubopt(char** optionp, char* const* tokens, char** valuep) -{ - char* option = *optionp; - - if ('\0' == *option) - return -1; - - char* end = strchr(option, ','); - - if (NULL == end) - end = option + strlen(option); - - char* equals = memchr(option, '=', (size_t)(end - option)); - size_t len = (size_t)((NULL != equals ? equals : end) - option); - - *optionp = ('\0' != *end) ? end + 1 : end; - - if ('\0' != *end) - *end = '\0'; - - for (int i = 0; NULL != tokens[i]; i++) { - - if ((0 == strncmp(option, tokens[i], len)) && ('\0' == tokens[i][len])) { - - *valuep = (NULL != equals) ? equals + 1 : NULL; - return i; - } - } - - *valuep = option; - return -1; -} - -ssize_t bartorch_no_readlink(const char* path, char* buf, size_t size) -{ - (void)path; - (void)buf; - (void)size; - - errno = ENOSYS; - return -1; -} diff --git a/src/csrc/substitute/posix.h b/src/csrc/substitute/posix.h deleted file mode 100644 index fc9fe133..00000000 --- a/src/csrc/substitute/posix.h +++ /dev/null @@ -1,15 +0,0 @@ -/* - * Declarations of what src/csrc/substitute/posix.c provides, included into - * every translation unit on Windows: GCC 14 refuses a call to an undeclared - * function, and MinGW's headers declare neither. - */ -#ifndef BARTORCH_SUBSTITUTE_POSIX_H -#define BARTORCH_SUBSTITUTE_POSIX_H - -#include -#include - -extern int getsubopt(char** optionp, char* const* tokens, char** valuep); -extern ssize_t bartorch_no_readlink(const char* path, char* buf, size_t size); - -#endif