From f9366e2ffcd04d45c8461df323ccbc06360317a1 Mon Sep 17 00:00:00 2001 From: mmelnich Date: Wed, 5 Aug 2026 17:06:28 -0700 Subject: [PATCH 1/2] MSVC OpenMP: /openmp:experimental -> /openmp:llvm, and respect overrides The classic and experimental MSVC OpenMP modes implement OpenMP 2.0, which limits parallel-for indices to signed 32-bit integers. RandBLAS consumers -- RandLAPACK in particular -- parallelize loops with int64_t indices and use the collapse clause, both of which only compile under the /openmp:llvm runtime. That runtime also supports the omp simd directive the sparse kernels use, so nothing here regresses. Also fixes an un-overridability bug: the post-find_package re-stamp of OpenMP::OpenMP_CXX hardcoded the mode, so passing -DOpenMP_CXX_FLAGS= at configure time silently lost. The requested flags are now captured before the find and re-applied afterward. --- CMake/OpenMP.cmake | 20 +++++++++++++------- CMake/RandBLASConfig.cmake.in | 18 ++++++++++++------ 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/CMake/OpenMP.cmake b/CMake/OpenMP.cmake index 09e73c61..b65a0850 100644 --- a/CMake/OpenMP.cmake +++ b/CMake/OpenMP.cmake @@ -1,21 +1,27 @@ message(STATUS "Checking for OpenMP ... ") -# MSVC's ordinary /openmp mode does not support the omp simd directive used by -# the sparse kernels. Recent MSVC versions provide it through -# /openmp:experimental. Allow callers to override this setting explicitly. +# MSVC's classic /openmp mode implements OpenMP 2.0 only: it rejects the +# omp simd directive used by the sparse kernels, 64-bit loop indices, and +# the collapse clause used by downstream consumers such as RandLAPACK. The +# /openmp:llvm runtime supports all of these (and subsumes what +# /openmp:experimental offered). Callers can still override the mode +# explicitly with -DOpenMP_CXX_FLAGS=... at configure time. if (MSVC AND NOT DEFINED OpenMP_CXX_FLAGS) - set(OpenMP_CXX_FLAGS "/openmp:experimental" CACHE STRING + set(OpenMP_CXX_FLAGS "/openmp:llvm" CACHE STRING "OpenMP compiler flags for C++") endif() +if (MSVC) + set(RandBLAS_OpenMP_MSVC_FLAGS "${OpenMP_CXX_FLAGS}") +endif() find_package(OpenMP COMPONENTS CXX) # FindOpenMP may replace OpenMP_CXX_FLAGS while probing the compiler. Ensure -# the imported target used by RandBLAS carries the SIMD-capable MSVC option. +# the imported target used by RandBLAS carries the requested MSVC mode. if (MSVC AND OpenMP_CXX_FOUND AND TARGET OpenMP::OpenMP_CXX) set_property(TARGET OpenMP::OpenMP_CXX PROPERTY - INTERFACE_COMPILE_OPTIONS "/openmp:experimental") - set(OpenMP_CXX_FLAGS "/openmp:experimental") + INTERFACE_COMPILE_OPTIONS "${RandBLAS_OpenMP_MSVC_FLAGS}") + set(OpenMP_CXX_FLAGS "${RandBLAS_OpenMP_MSVC_FLAGS}") endif() set(tmp FALSE) diff --git a/CMake/RandBLASConfig.cmake.in b/CMake/RandBLASConfig.cmake.in index 858648f8..3ac3f89a 100644 --- a/CMake/RandBLASConfig.cmake.in +++ b/CMake/RandBLASConfig.cmake.in @@ -23,21 +23,27 @@ find_dependency(Random123) # OpenMP set(RandBLAS_HAS_OpenMP @RandBLAS_HAS_OpenMP@) if (RandBLAS_HAS_OpenMP) - # MSVC's standard /openmp mode does not support the omp simd directive - # used by RandBLAS's sparse kernels. + # MSVC's classic /openmp mode implements OpenMP 2.0 only: it rejects the + # omp simd directive used by RandBLAS's sparse kernels, 64-bit loop + # indices, and the collapse clause used by downstream consumers. The + # /openmp:llvm runtime supports all of these. Consumers can still + # override the mode explicitly with -DOpenMP_CXX_FLAGS=... if (MSVC AND NOT DEFINED OpenMP_CXX_FLAGS) - set(OpenMP_CXX_FLAGS "/openmp:experimental" CACHE STRING + set(OpenMP_CXX_FLAGS "/openmp:llvm" CACHE STRING "OpenMP compiler flags for C++") endif() + if (MSVC) + set(RandBLAS_OpenMP_MSVC_FLAGS "${OpenMP_CXX_FLAGS}") + endif() find_dependency(OpenMP COMPONENTS CXX) # FindOpenMP may retain a previously detected /openmp setting. Ensure that - # consumers of the installed package receive the SIMD-capable MSVC mode. + # consumers of the installed package receive the requested MSVC mode. if (MSVC AND OpenMP_CXX_FOUND AND TARGET OpenMP::OpenMP_CXX) set_property(TARGET OpenMP::OpenMP_CXX PROPERTY - INTERFACE_COMPILE_OPTIONS "/openmp:experimental") - set(OpenMP_CXX_FLAGS "/openmp:experimental") + INTERFACE_COMPILE_OPTIONS "${RandBLAS_OpenMP_MSVC_FLAGS}") + set(OpenMP_CXX_FLAGS "${RandBLAS_OpenMP_MSVC_FLAGS}") endif() endif() From c72a1a7707467165ec72563e18b91dd3cafd97c5 Mon Sep 17 00:00:00 2001 From: mmelnich Date: Wed, 5 Aug 2026 19:43:56 -0700 Subject: [PATCH 2/2] Skip the omp simd directive on MSVC CI answered the open question from the /openmp:llvm switch: MSVC's llvm mode rejects the simd directive outright (error C7660), while the experimental mode accepted it but ignored the reduction clause without vectorizing the loop -- the directive has never bought MSVC anything. Compile it only for compilers whose OpenMP actually implements it, and keep /openmp:llvm as the MSVC mode (64-bit loop indices and collapse, which downstream consumers need). --- RandBLAS/sparse_data/csr_spmm_impl.hh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RandBLAS/sparse_data/csr_spmm_impl.hh b/RandBLAS/sparse_data/csr_spmm_impl.hh index 0705bd4c..39620e8f 100644 --- a/RandBLAS/sparse_data/csr_spmm_impl.hh +++ b/RandBLAS/sparse_data/csr_spmm_impl.hh @@ -65,7 +65,12 @@ static void apply_csr_to_vector_ik_impl( // ^ silence compiler complaints if UnitStride == true. for (int64_t i = 0; i < len_Av; ++i) { T Av_i_diff = 0.0; + // MSVC's /openmp:llvm mode rejects the simd directive (C7660), and its + // /openmp:experimental mode ignores the reduction clause without + // vectorizing -- so on MSVC this pragma buys nothing either way. + #if !defined(_MSC_VER) #pragma omp simd reduction(+:Av_i_diff) + #endif for (int64_t ell = rowptr[i]; ell < rowptr[i+1]; ++ell) { int64_t j = colidxs[ell]; if constexpr (UnitStride) {