Repository navigation
Add missing direct includes for MSVC builds (stdint.h, complex) - #87
Merged
Merged
Conversation
config.h uses int64_t for lapack_int under LAPACK_ILP64 but never includes <stdint.h>; lartg.cc uses std::complex but never includes <complex>. Both compile on Unix only through transitive includes of the platform C/C++ standard library; MSVC's headers do not provide them, so native MSVC ILP64 builds fail (C4430/C2146 in config.h, C2039 in lartg.cc). Include both headers directly. Reported in RandBLAS Windows-portability work by Raphael A. Meyer.
mmelnich
added a commit
to BallisticLA/RandLAPACK
that referenced
this pull request
Aug 12, 2026
References like "blaspp #132" and "lapackpp #87" were written bare. GitHub resolves a bare #NNN against the repository it is rendered in, so these linked to RandLAPACK #132 and #87 -- unrelated pull requests (funNystrom++ and a benchmarking DNM) -- rather than to the upstream fixes they name. A reader following them lands somewhere plausible and wrong, which is worse than a dead link. Now written as icl-utk-edu/blaspp#132 and icl-utk-edu/lapackpp#87, which GitHub renders as cross-repository links. Same for the other upstream references in these files.
mmelnich
added a commit
to BallisticLA/RandLAPACK
that referenced
this pull request
Aug 12, 2026
## Problem A Windows install failed on a plain Visual Studio machine. Fixing it uncovered **three independent causes**, all invisible to CI for the same reason: CI never runs the configuration a user actually has. **1. vcpkg.** The installer ran a classic-mode `vcpkg install`, but the vcpkg bundled with Visual Studio is manifest-only. CI runners ship a separate, classic-capable vcpkg. **2. A 32-bit toolchain.** BLAS++ reported `BLAS library not found` while oneMKL was correctly installed *and discovered*. The real cause is one line earlier: `.../bin/Hostx86/x86/cl.exe` — a 32-bit compiler, whose linker cannot use an x64 import library. Our documentation caused it: it said to open "Developer PowerShell for VS 2022", which defaults to **x86**. "Developer Command Prompt" is a *64-bit process* that also defaults to x86, so shell bitness is not a usable signal — only "x64 Native Tools Command Prompt" gives an x64 toolchain. CI sets `arch: x64` explicitly, so it never ran the prescribed shell. **3. Spaces in the library path.** With a correct x64 toolchain *and* our own link check passing, BLAS++ **still** failed. Its `try_compile` log gives the reason: ``` ninja: error: 'C:/Program', needed by 'cmTC_x.exe', missing and no known rule to make it ``` BLAS++ flattens `BLAS_LIBRARIES` from a CMake list into a space-separated string, then splits it back on spaces before probing. That round-trip is lossy: once joined, a space *inside* a path is indistinguishable from a separator. Intel installs oneMKL to `C:\Program Files (x86)\...` by default, so **every discovered oneMKL hits this**. It stayed hidden because the *downloaded* oneMKL lands in a space-free directory — the layout CI uses. ## Library discovery and provisioning Windows has **no system prefix for third-party libraries, no loader cache, no RPATH**. So build-time discovery cannot be a filesystem search (CMake's own `FindBLAS` finds MKL only via `MKLROOT`), and at run time Windows searches the executable's own directory **first** and `PATH` **last**. PATH is the wrong tool in both phases. **`-Backend mkl` (default)** is the one backend with real discovery, since oneMKL has a canonical location. Probed in order, first match wins: | # | Source | |---|--------| | 1 | `-MklRoot <path>` — explicit; invalid is a hard error, never a silent fallback | | 2 | `$env:MKLROOT` — set by `setvars.bat`; the variable CMake's `FindBLAS` uses | | 3 | `$env:ONEAPI_ROOT\mkl\latest` | | 4 | `C:\Program Files (x86)\Intel\oneAPI\mkl\latest` — the installer default | A candidate counts only if it holds `mkl_intel_ilp64_dll.lib` under `lib\` or `lib\intel64\` alongside a DLL directory (`bin\`, or `redist\intel64\` pre-2024), so a partial install is rejected rather than half-used. If nothing is found the installer states what it searched and **asks** before downloading a pinned, checksum-verified copy into the project directory; declining lists the alternatives and exits non-zero. `-NoDownload` turns "not found" into an error outright. **`-Backend openblas`** gets no discovery deliberately — no canonical location exists (GitHub zips, vcpkg, conda and MSYS2 all differ, and the release zips ship configs with wrong hardcoded paths). It asks whether you already have OpenBLAS and, if so, prints the exact `-Backend custom` invocation. **`-Backend custom`** takes your `.lib` paths and DLL directory — the route for AMD AOCL, whose downloads are licence-gated. Whatever the source, libraries are proved to work **before any dependency is built**, by compiling, linking and *running* a `dgemm_`/`dgesv_` program with a numeric check. That check previously skipped the default `mkl` backend, which is why cause 2 surfaced three layers down as a misleading BLAS error. Where paths contain spaces, import libraries are staged into a space-free directory. That is a workaround: the underlying bug is fixed upstream in [icl-utk-edu/blaspp#137](icl-utk-edu/blaspp#137), and removing the staging once that lands is tracked in #158. Prompts appear only when someone can answer them: `$script:Interactive` is false whenever stdin is redirected, mirroring `install.sh`'s `INTERACTIVE` flag. Every question has a defensible unattended default, so CI cannot hang. ## What this PR does 1. **vcpkg removed.** oneMKL comes from Intel's official NuGet packages, pinned by version and SHA256. No package manager prerequisite. 2. **Backend choice** — `-Backend mkl|openblas|custom` — with the discovery, provisioning and validation above, plus `-NoDownload` / `-Yes`. 3. **x64 toolchain guard** in both `install.ps1` preflight and `setup.ps1`, with *different* messages for x86 (wrong shell, one-command fix) and arm64/arm (unsupported — no oneMKL build exists). Shared via `.github/scripts/windows/toolchain-arch.ps1`; reads `VSCMD_ARG_TGT_ARCH`, then the `bin\Host<host>\<target>\` convention, then `cl.exe`'s banner — the banner alone would miss on a localized Visual Studio, and a missed detection fails *open*. 4. **No PATH edits, ever.** Runtime DLLs are staged beside each executable (app-local deployment). `RANDLAPACK_RUNTIME_DLL_DIRS` covers the backend DLLs `TARGET_RUNTIME_DLLS` cannot see; the installed package exports `randlapack_stage_runtime_dlls()` for downstream projects. 5. **MSVC OpenMP fixed, and now reaching users.** RandLAPACK's `find_package(OpenMP)` ran before RandBLAS's `/openmp:llvm` guard, so classic `-openmp` was cached — under which MSVC silently ignores the `collapse` clause `rl_rpchol` needs (C4849). The installer also unconditionally disabled OpenMP, so the fix shipped unusable; it is now on by default, with `-NoOpenMP` to opt out. 6. **Dependencies from upstream, pinned to immutable refs.** BLAS++/LAPACK++ came from forks carrying two one-line MSVC fixes; both merged upstream 2026-08-06 (icl-utk-edu/blaspp#132, icl-utk-edu/lapackpp#87), so both now come from `icl-utk-edu`. They were pinned to *branch names* inside a cache keyed on the setup script, so a cache hit could restore a different revision than a miss builds. Everything the installer can fetch is now the **newest stable release, pinned to an exact version**: oneMKL `2026.1.0.226`, OpenBLAS `0.3.34`, GoogleTest `v1.18.0`, Random123 `v1.14.0`. BLAS++ `3057185` and LAPACK++ `40b9d0d` are the two exceptions, pinned to commits because the latest release of each (`v2025.05.28`) predates the MSVC fixes; they move to a tag once one carries them. The oneMKL bump renames the runtime DLLs `mkl_*.2.dll` -> `mkl_*.3.dll`; nothing hardcodes those names (staging globs `*.dll`), and both provisioning paths were re-verified after the bump. 7. **Reuse gated on provenance, not presence.** A clone is reused only if at the pinned remote and ref; a built dependency only if built from the source we would build from now. Without this, changing a pin is a no-op for anyone who already has an install. 8. **One CI job, `windows-toolchain-guards`,** covering the documented user path the build matrix structurally cannot. It asserts refusals and runs pure logic, so nothing builds and it finishes in seconds. A decision table covers `arm64`/`arm` — the only possible coverage, since we cannot build for them — and integration steps run the installer under a real x86 toolchain and an `amd64_arm64` cross-compiling one, giving an arm64-targeting `cl.exe` on an x64 runner. It launches exactly as the docs prescribe, so the documented invocation stays under test. 9. **CI ran every job twice.** All four workflows fired on `pull_request` *and* on `push` for every branch, so each commit ran the full matrix twice — same SHA, same result, 22 check runs where 11 would do. `push` is now restricted to `main`. Nothing cancelled superseded runs either, so pushing a fix left the previous run going to completion; a `concurrency` group now supersedes in-flight runs, for pull requests only (on `main` every commit should still be validated). This removes automatic CI for a branch with no pull request open, which `workflow_dispatch` covers on demand. 10. **Smaller fixes:** Ninja unified across dependency builds; `--retry-all-errors` on downloads (plain `--retry` misses connection-level failures like curl 52); cache keys off `hashFiles()`, which silently resolves EMPTY under the install-script checkout path and split caches that claimed to be shared; and a dead `ctest` exclusion for `TestABRIK.ABRIK_catch_instability` — a test gone since January 2025 — removed from the Windows paths, where it had leaked into the user-facing installer even though `install.sh` has no such exclusion. 11. **Docs.** New [INSTALL_WINDOWS.md](INSTALL_WINDOWS.md): quick start, Windows-vs-Unix contrast, backend table, `install.ps1` reference, runtime-DLL explainer, troubleshooting. It states the toolchain contract the way Linux and macOS do -- you bring a compiler, CMake, Ninja and Git, in whatever terminal you like -- and then offers ways to satisfy it rather than mandating one shell, since naming a single Start-menu entry without naming the requirement is what let the wrong shell go unnoticed. It verifies with `where cl` (bare `cl` is silent in both failing cases), gives a `vswhere -latest -products *` one-liner that works for any edition (without `-products *` it finds nothing on Build Tools), and documents `-ExecutionPolicy Bypass`, since stock Windows refuses to run `.ps1` at all. ## Verification All Windows CI green. Every backend and both interactive branches were also exercised on a machine that began with **no Visual Studio, CMake, Git, Ninja or MKL**, under Windows PowerShell 5.1 rather than CI's PowerShell 7, and later against a real oneAPI install at the default (spaced) location: | Path | Result | |---|---| | oneMKL **discovery** (the failing real-world case) | 749/749 | | oneMKL downloaded, from scratch | 745/745 serial, 749/749 OpenMP | | Build from the **upstream pins** | 749/749, origins confirmed `icl-utk-edu` | | `-Backend openblas` | 745/745 | | `-MklRoot` valid / invalid | reused / hard error | | `-Backend custom`, insufficient library | rejected (`LNK2019: unresolved dgemm_`) | | openblas interactive yes / no | custom recipe / downloads | | `-NoDownload` (mkl, openblas, declined) | all error, nothing fetched | | Real x86 toolchain | refused at preflight | | Guard decision table | 5/5, plus a mutation test catching a sabotaged guard | | Stale provenance | rebuilds; reuses on second run; fork clone re-cloned | | After bumping oneMKL to 2026.1.0.226 and GoogleTest to v1.18.0 | discovery 749/749; forced NuGet download verified both SHA256s and staged the renamed `mkl_*.3.dll` | The rows above the last one were measured before the version bump; the bump was then re-verified on both provisioning paths, which is the last row. Rebased onto `main` after #157, which quarantined the macOS Accelerate `gesdd` canary, so `core-macos` is green here rather than carrying a known failure. ## Notes for reviewers - The `amd64_arm64` leg is the only thing exercising a genuine ARM64 compiler; it could not be verified locally (adding the toolset needs interactive elevation) and passed on its first run. - The space-in-path staging works around a BLAS++ quoting bug; fixing that upstream would remove the need for it. - Pre-existing and left for separate changes: `install.sh` clones BLAS++/LAPACK++ at floating HEAD, and the same dead `ctest` exclusion remains in the three Linux/macOS workflows.
mmelnich
added a commit
to BallisticLA/RandBLAS
that referenced
this pull request
Aug 13, 2026
Completes the installer pair. install.ps1 produces the same RandNLA-project layout as install.sh, honours RANDNLA_PROJECT_DIR with the same precedence, and delegates dependency provisioning to the setup script CI already uses so there is one implementation rather than two that drift. The x64 toolchain guard is the reason this exists in the form it does. "Developer PowerShell for VS" and "Developer Command Prompt for VS" both default to an *x86* toolchain, and an x86 linker cannot use the x64 import libraries every BLAS backend ships. Left unchecked the failure surfaces three layers down as BLAS++ reporting "BLAS library not found", blaming the libraries when the compiler is at fault -- which is exactly how this was diagnosed in RandLAPACK. toolchain-arch.ps1 reads VSCMD_ARG_TGT_ARCH, then the bin\Host<host>\<target>\ convention, then cl.exe's banner, and refuses x86 and arm64 with different messages because they need different answers. Provisioner changes: * Off personal forks. BLAS++ and LAPACK++ came from RaphaelArkadyMeyerNYU/*; both MSVC fixes merged upstream on 2026-08-06 (icl-utk-edu/blaspp#132, icl-utk-edu/lapackpp#87), so both now come from icl-utk-edu pinned to the merge commits. * Pinned and provenance-stamped. Clone-Head took a branch name and returned early whenever the destination merely existed, so a branch tip could move between runs and changing a ref was a silent no-op for anyone who already had the directory. Random123 in particular was fetched at the default branch, unpinned. Clone-Pinned fetches one ref and records it. Also exports randblas_stage_runtime_dlls() from the installed package. Windows searches an executable's own directory first and PATH last, so a downstream project linking installed RandBLAS could not find the BLAS DLLs at run time -- the function existed only in the build tree. Found because the installer's own verification step is such a consumer and could not configure without it. Verified on Windows 11 with Windows PowerShell 5.1 and VS 2022 Build Tools: missing-prerequisite path, x86 toolchain refused at preflight under a real vcvars32 environment, and a full x64 install from scratch -- oneMKL through vcpkg, BLAS++, GoogleTest, RandBLAS -- ending with the verification program compiling, linking, staging its DLLs and running, reporting ILP64.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Building LAPACK++ natively with MSVC fails in the ILP64 configuration with diagnostics beginning:
Two independent missing direct includes:
include/lapack/config.huses globalint64_tforlapack_intwhenLAPACK_ILP64is enabled, but does not include<stdint.h>. Unix standard-library headers expose the type transitively through<stdlib.h>; MSVC's does not. TheLAPACK_S_SELECT2/ callback errors cascade from the failedlapack_inttypedef.src/lartg.ccusesstd::complexbut does not include<complex>. Under MSVC, the_MSC_VERbranch inlapack/config.hdefines C-compatible complex structs and never pulls in the C++<complex>header.This PR includes both headers directly rather than relying on transitive implementation details of a particular standard library.
Context: found during the RandBLAS native-Windows port (BallisticLA/RandBLAS#179) by Raphael A. Meyer (@RaphaelArkadyMeyerNYU); RandBLAS Windows CI (MSVC 19.51, oneMKL ILP64 sequential, NMake) has been building LAPACK++ with exactly these two includes since 2026-07 via a temporary fork branch. Environment tested there: Windows x64, MSVC 19.51, C++17, LAPACK++ 2025.05.28 (commit 8b32767).