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

BUG: Cannot build C++ project using MSVC 2019 and OpenBLAS #3661

Closed
larsoner opened this issue Jun 21, 2022 · 13 comments
Closed

BUG: Cannot build C++ project using MSVC 2019 and OpenBLAS #3661

larsoner opened this issue Jun 21, 2022 · 13 comments

Comments

@larsoner
Copy link
Contributor

larsoner commented Jun 21, 2022

On VS2019, trying to build using MSVC I get errors like the following:

[1/86] Building CXX object OpenMEEGMaths\CMakeFiles\OpenMEEGMaths.dir\src\vector.cpp.obj
FAILED: OpenMEEGMaths/CMakeFiles/OpenMEEGMaths.dir/src/vector.cpp.obj 
C:\PROGRA~2\MICROS~1\2019\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe  /nologo /TP -DH5_BUILT_AS_DYNAMIC_LIB -DHAVE_BLAS -DHAVE_LAPACK -DHAVE_SHARED_PTR_ARRAY_SUPPORT -DOpenMEEGMaths_EXPORTS -DUSE_OPENBLAS -I%SRC_DIR%\OpenMEEGMaths\include -I%SRC_DIR%\build_Release\OpenMEEGMaths -I%SRC_DIR%\build_Release -I%SRC_DIR%\OpenMEEGMaths\OpenMEEGMaths\src -external:I%PREFIX%\Library\include -external:W0 /wd4275 /wd4101 -openmp /MD /O2 /Ob2 /DNDEBUG -std:c++17 /showIncludes /FoOpenMEEGMaths\CMakeFiles\OpenMEEGMaths.dir\src\vector.cpp.obj /FdOpenMEEGMaths\CMakeFiles\OpenMEEGMaths.dir\ /FS -c %SRC_DIR%\OpenMEEGMaths\src\vector.cpp
%PREFIX%\Library\include\lapack.h(104): error C2143: syntax error: missing ',' before '*'
%PREFIX%\Library\include\lapack.h(106): error C2143: syntax error: missing ',' before '*'
... <hundreds of these>

You can see it on conda-forge here for example, where it uses the OpenBLAS libraries that they build:

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=524975&view=logs&jobId=5be07ae1-d8ba-5406-47b6-8e3a3a12f825&j=5be07ae1-d8ba-5406-47b6-8e3a3a12f825&t=0bf03e01-0bec-5b85-5316-b1633322e895

And on GitHub actions here from earlier this week, same error -- this is using the OpenBLAS builds SciPy makes with mingw64, but I had the same problem when I tried previously just by downloading the 0.3.20 release binaries from GitHub:

Run cmake --build build  --config Release
Microsoft (R) Build Engine version 16.11.2+f32259642 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.
  Checking Build System
  Building Custom Rule D:/a/openmeeg/openmeeg/OpenMEEGMaths/CMakeLists.txt
  vector.cpp
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.16.27023\include\xlocale(319): warning C4[53](https://github.com/openmeeg/openmeeg/runs/6975816876?check_suite_focus=true#step:16:54)0: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc [D:\a\openmeeg\openmeeg\build\OpenMEEGMaths\OpenMEEGMaths.vcxproj]
d:\a\openmeeg\openmeeg\openblas\64\include\lapack.h(104): error C2143: syntax error: missing ',' before '*' [D:\a\openmeeg\openmeeg\build\OpenMEEGMaths\OpenMEEGMaths.vcxproj]
d:\a\openmeeg\openmeeg\openblas\64\include\lapack.h(106): error C2143: syntax error: missing ',' before '*' [D:\a\openmeeg\openmeeg\build\OpenMEEGMaths\OpenMEEGMaths.vcxproj]
... <hundreds of these>

I am a bit mystified by this because these lines are protected by an ifdef that should take care of this I think:

https://github.com/xianyi/OpenBLAS/blob/9283c7c0b5a9ec7bbe3b6dfc1a019b29b3e112e5/lapack-netlib/LAPACKE/include/lapack.h#L71-L104

Anyone run into this before? I didn't see any mention of anything like this in the Visual Studio wiki docs, but maybe I just missed it. Perhaps it's a cmake+MSVC+CPP issue, and there is some cmake fix...? Not sure.

An identical build configuration using mingw64 doesn't have this problem... but I think for conda-forge I might need to build with MSVC. And in any case it would be nice if it worked...

@brada4
Copy link
Contributor

brada4 commented Jun 22, 2022

Where did you get that lapack.h from?

@larsoner
Copy link
Contributor Author

The include directory of the install step for OpenBLAS

@martin-frbg
Copy link
Collaborator

Same as what is in Reference-LAPACK actually. No idea why VS2019 does not like it, I'm fairly sure earlier versions did.

@larsoner
Copy link
Contributor Author

Okay I opened a near-duplicate issue at Reference-LAPACK/lapack#683

@martin-frbg
Copy link
Collaborator

I guess you could try replacing the float _Complex in the definition of lapack_complex_float on line 39 with the Microsoft equivalent _Fcomplex (came across this when preparing the updated f2clapack that is now in the develop branch)
https://docs.microsoft.com/en-us/cpp/c-runtime-library/complex-math-support?view=msvc-170

@isuruf
Copy link
Contributor

isuruf commented Jun 23, 2022

Not sure why we have this define at https://github.com/xianyi/OpenBLAS/blob/ad4598143bfbf77acd16f4ae884462edc69b496e/lapack-netlib/LAPACKE/include/lapack.h#L7
If that is removed and lapacke_config.h is included unconditionally, I think MSVC should be able to process the header just fine.

@martin-frbg
Copy link
Collaborator

File history suggests it has been this way ever since LAPACKE got integrated into the LAPACK package in 2011 - maybe the ifdef made sense when it was separate

@martin-frbg
Copy link
Collaborator

Also lapacke_config.h woud probably need a define LAPACK_COMPLEX_STRUCTURE dependent on (some variant spelling of) C_MSVC like there already is for older Android. (At least I see nothing defining this elsewhere, and without it we are back at float _Complex vs. _Fcomplex)

@martin-frbg
Copy link
Collaborator

@larsoner could you try @isuruf 's suggestion ?

@larsoner
Copy link
Contributor Author

larsoner commented Jul 5, 2022

When I take the 0.3.18 headers and comment out lines 7 and 9 (#if and #endif) I still get a ton of errors like

  vector.cpp
c:\users\larsoner\python\openmeeg\openblas\64\include\lapacke_config.h(109): error C2146: syntax error: missing ';' bef
ore identifier 'lapack_make_complex_float' [C:\Users\larsoner\python\openmeeg\build\OpenMEEGMaths\OpenMEEGMaths.vcxproj
]
c:\users\larsoner\python\openmeeg\openblas\64\include\lapacke_config.h(109): error C4430: missing type specifier - int
assumed. Note: C++ does not support default-int [C:\Users\larsoner\python\openmeeg\build\OpenMEEGMaths\OpenMEEGMaths.vc
xproj]
c:\users\larsoner\python\openmeeg\openblas\64\include\lapacke_config.h(110): error C2371: '_Complex': redefinition; dif
ferent basic types [C:\Users\larsoner\python\openmeeg\build\OpenMEEGMaths\OpenMEEGMaths.vcxproj]

One other thing that might be relevant is this in the CMAKE config of the OpenMEEG project:

CMakeLists.txt:set(CMAKE_CXX_STANDARD 17)  # use c++17

so maybe this hasn't been noticed because nobody has compiled with MSVC using the c++17 standard...?

@brada4
Copy link
Contributor

brada4 commented Jul 6, 2022

@larsoner
Copy link
Contributor Author

larsoner commented Jul 6, 2022

😱 I hope this issue doesn't turn out to be something like that where I missed something in OpenMEEG itself

I'll check next time I'm on my Windows boot...

@larsoner
Copy link
Contributor Author

larsoner commented Jul 8, 2022

Okay I changed the lines @brada4 pointed out from their original form:

#if WIN32
    #define LAPACK_COMPLEX_CUSTOM
    #define lapack_complex_float float
    #define lapack_complex_double double
#endif

which show the problem on CIs in openmeeg/openmeeg#454, to a variant of what @martin-frbg originally suggested:

#if defined(_MSC_VER)
    #include <complex.h>
    #define LAPACK_COMPLEX_CUSTOM
    #define lapack_complex_float _Fcomplex
    #define lapack_complex_double _Dcomplex
#endif

And things compile fine with openmeeg/openmeeg@6eb29e8 locally with MSVC 2017 at least and tests pass, though I still need to convince MSVC 2019 in CIs to accept it! 🚀

I'll close this as fixed, and I've updated the MSVC instruction page to mention this: https://github.com/xianyi/OpenBLAS/wiki/How-to-use-OpenBLAS-in-Microsoft-Visual-Studio#visual-studio-2017-c2017-standard

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

No branches or pull requests

4 participants