Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GitHub-CI: Add MSVC runners #4270

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

mmuetzel
Copy link
Contributor

@mmuetzel mmuetzel commented Oct 18, 2023

This adds GitHub hosted runners using the MSVC compiler on Windows.

I tried to build with -DDYNAMIC_ARCH=ON. But that failed with:

C:\PROGRA~1\MICROS~2\2022\ENTERP~1\VC\Tools\MSVC\1435~1.322\bin\HostX64\x64\cl.exe  /nologo -DCMAKE_INTDIR=\"Debug\" -ID:/a/OpenBLAS/OpenBLAS -ID:/a/OpenBLAS/OpenBLAS/build /DWIN32 /D_WINDOWS /W3  -openmp -DUSE_OPENMP -DF_INTERFACE_GFORT -DSMALL_MATRIX_OPT -DDYNAMIC_ARCH -DNO_AVX512 -DSMP_SERVER -DNO_WARMUP -DMAX_CPU_NUMBER=64 -DMAX_PARALLEL_NUMBER=1 -DNO_AFFINITY -DVERSION="\"0.3.24.dev\"" -DBUILD_SINGLE -DBUILD_DOUBLE -DBUILD_COMPLEX -DBUILD_COMPLEX16 /MDd /Zi /Ob0 /Od /RTC1 /showIncludes /Fodriver/level3/CMakeFiles/driver_level3.dir/Debug/CMakeFiles/csyrk_kernel_U.c.obj /Fddriver\level3\CMakeFiles\driver_level3.dir\Debug\ /FS -c D:/a/OpenBLAS/OpenBLAS/build/driver/level3/CMakeFiles/csyrk_kernel_U.c
D:/a/OpenBLAS/OpenBLAS/driver/level3/syrk_kernel.c(65): error C2057: expected constant expression
D:/a/OpenBLAS/OpenBLAS/driver/level3/syrk_kernel.c(65): error C2466: cannot allocate an array of constant size 0
D:/a/OpenBLAS/OpenBLAS/driver/level3/syrk_kernel.c(65): error C2133: 'subbuffer': unknown size

I don't know if that means that there is an issue with DYNAMIC_ARCH in general for that compiler or if that is caused by something else.
I also don't know if -DTARGET=CORE2 is a good choice in that case. Please, let me know if that should be changed.

Since I ended up not building with DYNAMIC_ARCH, I didn't add these rules to dynamic_arch.yaml.

CMake warned during configure that building shared and static libraries at the same time is unsupported. So, I opted for building both separately in a matrix. (Do you know why that is unsupported?)

I didn't try yet to figure out how to bring a Fortran compiler into this mix. It's building with -DC_LAPACK=ON instead.

I see a different test is run on Azure for Windows_cl (openblas_utest.exe). Should that be run here, too?

@martin-frbg
Copy link
Collaborator

What's the point here exactly ? DYNAMIC_ARCH is useless with MSVC as that compiler does not support the assembly kernels. Though the compile error you saw comes earlier and is probably a result of MSVC not supporting the indirections that define GEMM_UNROLL_MN for a particular target, and/or computation of a constant from preprocessor variables.
And the git blame output for CMakelists.txt reveals that the MSVC-specific restriction on building both static&shared in one go is from #3411, where isuruf cautioned in his review of the PR that both libraries would get the same name with MSVC.

@mmuetzel
Copy link
Contributor Author

Sorry if this came across as a bug report. It was meant to be a "proper" PR.
I gathered from your comments in #4263 that you'd like to move more of the CI to GitHub. This is meant as an attempt in doing that.

I started with build rules that were pretty similar to the ones used for MinGW (with MSYS2). My comments were meant to describe why there are differences. Sorry for not making that clearer before.

Thank you for answering the questions I had.

If I misunderstood what you meant in #4263, feel free to close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants