Make HPTT buildable with MSVC - #5
Merged
Merged
Conversation
`__restrict__` and `__attribute__((always_inline))` are GNU spellings. MSVC accepts neither: it spells the restrict qualifier `__restrict` and forced inlining `__forceinline`, so every declaration carrying one of these failed to parse under cl. Add HPTT_RESTRICT to hptt_types.h and use it in place of `__restrict__` throughout transpose.h and transpose.cpp, and give macros.h an INLINE definition for _MSC_VER. HPTT_RESTRICT lives in hptt_types.h rather than macros.h because transpose.h -- a public, installed header -- applies it to the A_ and B_ data members, and macros.h also defines the unprefixed `INLINE`, which should not leak into consumers' translation units. clang-cl defines _MSC_VER but understands the GNU spelling, so it is excluded from the MSVC branch. Co-Authored-By: Claude <noreply@anthropic.com>
The Transpose constructor sized four int buffers by the runtime `dim` argument, and getBestLoopOrder() sized its cost matrix by `dim_*dim_`. Variable-length arrays are a C99 feature that C++ never adopted; GCC and Clang accept them as an extension, MSVC rejects them with C2131. Use std::vector for all five and pass .data() at the call sites that expect a raw pointer. Co-Authored-By: Claude <noreply@anthropic.com>
cTensorTranspose() and zTensorTranspose() are declared with the C99 `_Complex` type specifier. MSVC does not support `_Complex` when compiling C++: in C++ mode <complex.h> resolves to the STL header, which forwards to <complex> and never declares the UCRT's _Fcomplex / _Dcomplex. cl therefore rejects both declarations with C2146/C3646/C2062. Nothing in HPTT's own sources includes hptt.h, so this only breaks consumers of the installed headers -- but for those it makes the public header unusable on MSVC entirely. There is no portable spelling for these two signatures, so guard them on compilers that understand `_Complex`. clang-cl defines _MSC_VER but does understand it, so it keeps the declarations. Co-Authored-By: Claude <noreply@anthropic.com>
ENABLE_AVX appended the GCC/Clang flag -mavx unconditionally. cl does not accept it; the MSVC equivalent is /arch:AVX. Select per compiler. ENABLE_ARM appended -mfpu=neon, which is an ARM32 flag with no MSVC counterpart -- on MSVC ARM64 Advanced SIMD is part of the baseline ISA and /arch: only selects newer extensions. Skip the flag there and keep defining HPTT_ARCH_ARM, which is what selects the Neon kernels and REGISTER_BITS=128. Co-Authored-By: Claude <noreply@anthropic.com>
hptt_dyn was built unconditionally. A parent project that embeds HPTT via add_subdirectory() and links only hptt_static still paid for a second full compile of every source, and on Windows it also inherited the shared target's artifact-naming constraints for a library it never uses. Add HPTT_BUILD_SHARED (default ON, so standalone builds are unchanged) and create hptt_dyn only when it is set. Collect the enabled targets in HPTT_TARGETS so export(TARGETS) and install(TARGETS) both follow the option without naming hptt_dyn directly. Co-Authored-By: Claude <noreply@anthropic.com>
No declaration in HPTT is annotated with __declspec(dllexport), so on Windows the DLL exported nothing. The linker then emitted no import library at all, which leaves install(TARGETS hptt_dyn ARCHIVE ...) with no file to install and gives consumers nothing to link against. Set WINDOWS_EXPORT_ALL_SYMBOLS on hptt_dyn. CMake generates the .def file from the object files, which covers the four explicit Transpose instantiations and the create_plan() overloads. The property is ignored on non-Windows platforms. Co-Authored-By: Claude <noreply@anthropic.com>
hptt_configure_target() sets OUTPUT_NAME to `hptt` for both targets. On
Windows a static library and a DLL's import library are both ARCHIVE
artifacts named <OUTPUT_NAME>.lib, so the two targets claimed the same
output path. The Ninja generator rejects the resulting build graph with
ninja: error: build.ninja:187: multiple rules generate hptt.lib
Set ARCHIVE_OUTPUT_NAME on hptt_dyn only. The DLL keeps the name
hptt.dll and the static archive keeps hptt.lib; just the import library
becomes hptt_dyn.lib. ARCHIVE_OUTPUT_NAME applies only to DLL-platform
shared libraries, so libhptt.a, libhptt.so and libhptt.dylib are
unaffected.
Co-Authored-By: Claude <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
This was referenced Jul 29, 2026
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
1 participant
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.
Makes HPTT configure and build with MSVC 2022 + Ninja. Addresses Cytnx-dev/Cytnx#1112.
Verified locally against cl 19.44.35228 (VS 2022 BuildTools 14.44.35207), CMake 4.4.1, Ninja 1.13.2.
The build failures, in order of discovery
ninja: error: build.ninja:187: multiple rules generate hptt.libARCHIVE_OUTPUT_NAME hptt_dynon the shared target only__restrict__rejectedHPTT_RESTRICT->__restricton MSVCINLINEundefined__forceinlinefor_MSC_VERstd::vector-mavxrejected/arch:AVXWINDOWS_EXPORT_ALL_SYMBOLShptt.hunparseable by consumers (_Complex)_ComplexexistsItem 7 was not in the issue list.
cTensorTranspose/zTensorTransposeare declared with the C99_Complexspecifier; MSVC has no_Complexin C++ mode, where<complex.h>resolves to the STL header and forwards to<complex>. None of HPTT's own sources includehptt.h, so this breaks only consumers of the installed headers — which is exactly how Cytnx uses HPTT.On the artifact naming
The issue offered two shapes. This PR takes the narrower one: rename only the import library. The DLL stays
hptt.dlland the static archive stayshptt.lib; only the colliding artifact becomeshptt_dyn.lib.ARCHIVE_OUTPUT_NAMEapplies only to DLL-platform shared libraries, solibhptt.a/libhptt.so/libhptt.dylibare untouched.HPTT_BUILD_SHAREDNew option, default
ON, so standalone builds are unchanged. A parent that links onlyhptt_staticcan set itOFFand skip the second full compile of every source.HPTT_TARGETScollects the enabled targets soexport(TARGETS)andinstall(TARGETS)follow the option.OpenMP:
/openmpis sufficient — no/openmp:llvmThe issue's plan implies OpenMP 3.x is needed. It isn't, and I checked before assuming:
HPTT_DUPLICATE_2— the onlycollapse(2)user — is never called anywhere in the tree.#pragma omp parallel for(transpose.cpp:840,transpose.cpp:1941,utils.cpp:81) uses anintinduction variable, so OpenMP 2.0's signed-index restriction (C3016) never bites.All four sources compile clean under plain
/openmp. This matters beyond tidiness:/openmp:llvmwould require redistributinglibomp140.x86_64.dll, whereas/openmp'svcomp140.dllalready ships inSystem32. No source or CMake change was needed here.Verification
Every configuration below was built and run:
hptt.lib,hptt.dll,hptt_dyn.libHPTT_BUILD_SHARED=OFFhptt.libonly, no DLLinstall+find_package(hptt CONFIG)consumer, statichptt::hptt_dynagainst the DLLadd_subdirectory+ static-only (Cytnx's integration mode)ENABLE_AVX=ON/arch:AVXpresent incompile_commands.jsonCorrectness was checked with a consumer that transposes random tensors against a naive index-arithmetic reference: all four instantiated scalar types, 2D through 5D, non-trivial permutations,
beta != 0, complexalpha/beta, and 1 and 4 threads. Every case matched bit-exactly (max|B-Bref| = 0), against both the static library and the DLL.Cross-platform verification
The CI added in #6 is based on this branch, so its run covers these commits. All six jobs pass: linux-x64, linux-arm64, macos-x64, macos-arm64, windows-x64, windows-arm64 (https://github.com/Cytnx-dev/hptt/actions/runs/30481654763). Each job builds HPTT and then runs the correctness consumer against both the build tree and an install tree.
One caveat: linux-arm64 is green only with an additional fix that lives in #6, not here. HPTT applies
-march=x86-64on every architecture, so GCC on aarch64 rejects it (unknown value 'x86-64' for '-march') before any of this branch's changes matter. That is a pre-existing bug unrelated to MSVC, so it is fixed there rather than in this PR. The other five platforms validate this branch as it stands.Pre-existing warnings, left alone
MSVC emits C4267/C4305/C4244 narrowing warnings and C4068 for the Intel-only
#pragma vector nontemporal. All predate this PR and none block the build.