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

Support compiling with clang on windows #1256

Merged
merged 19 commits into from
Aug 6, 2017

Conversation

isuruf
Copy link
Contributor

@isuruf isuruf commented Jul 29, 2017

In CMake, MSVC is set to 1 for clang when clang is targeting the MSVC runtime. So there were a few changes for that.

With clang, the assembly files can be compiled and therefore optimized kernel is enabled by default for clang on windows.

One issue I can't figure out is why utest/openblas_utest.exe gives the following

RESULTS: 0 tests (0 ok, 0 failed, 0 skipped)

Any ideas?

@brada4
Copy link
Contributor

brada4 commented Jul 29, 2017

You change cmake+clang build on linux, you should not.

@isuruf
Copy link
Contributor Author

isuruf commented Jul 29, 2017

@brada4, I don't think I have. Can you elaborate more?

@brada4
Copy link
Contributor

brada4 commented Jul 29, 2017

I understood, you compile with clang in vcvarsall.bat environment.
What do you intend to do for fortran part?

@martin-frbg
Copy link
Collaborator

Support for flang was added a few days ago, if this is also available on windows it would seem a natural choice ?

@isuruf
Copy link
Contributor Author

isuruf commented Jul 30, 2017

I haven't used flang on windows. They don't mention windows support.

@brada4
Copy link
Contributor

brada4 commented Jul 30, 2017

No flang on Windows.
The problem is that using mingw gfortran for lapack pulls some 4-5 file extra redist dlls from mingw.
There is no problem with mingw blas-only build that links only to msvcrt.dll, so this PR just another way to achieve same. But a support for another compiler is always nice.
Quite a great job sanitizing Complex number support between the lines.

@brada4
Copy link
Contributor

brada4 commented Jul 30, 2017

I just read what I wrote, i think gfortran unnecesarily has -lpthread in Windows build, I will check this theory tomorrow when I get to see Windows. Maybe one dll less if i am right

@martin-frbg
Copy link
Collaborator

I'm given to understand linking to mingw-built libraries is not always desirable in Windows projects due some kind of version conflict with/of CRT implementations ? In that case a non-mingw build that actually uses the optimized assembly kernels (unlike what you would currently get with VC, as that does not support inline assembly and AT&T-style assembly syntax) would probably be an improvement even if it is limited to (C)BLAS for now.
I take it that brada4' initial complaint was a misunderstanding, so that would leave your very own open question about the utest doing nothing - did this get fixed as part of your PR1259 ?

@isuruf
Copy link
Contributor Author

isuruf commented Aug 3, 2017

No, it didn't get fixed. It has to do with some issue in ctest that it can't find the tests using ctest "magic"

@brada4
Copy link
Contributor

brada4 commented Aug 3, 2017

I mistook vs clang with llvm installer, yes i am wrong,

@martin-frbg
Copy link
Collaborator

I'm not sure I even understand how ctest/utest is supposed to work. What I did notice is that ctest.h has
a few #ifdefs for various compilers that appear to influence its workings - perhaps a heavyhanded approach of just trying #define __CTEST_APPLE and #define __CTEST_MSVC in turn might uncover something ?

@brada4
Copy link
Contributor

brada4 commented Aug 3, 2017

Actually mingw libraries are better than msvc built ones as they do not link to particular version of msvcrt
i.e prebuilt binary is significantly less consumable - only on same msvcrt - than mingw one, but integrating OpenBLAS in own project and building along rest against current msvcrt (with known limitation of absent flang and lapack) still could be option for many.

@isuruf
Copy link
Contributor Author

isuruf commented Aug 4, 2017

I tried lots of ways to make ctest support clang on windows, but it can't find the tests automatically. So, I've added a file that adds the tests manually.

@isuruf
Copy link
Contributor Author

isuruf commented Aug 4, 2017

Travis failure is unrelated.

@martin-frbg
Copy link
Collaborator

Yes, linker task got killed with SIGKILL - could be it hit the 4mb logfile limit from all the type conversion and uninitialized variable warnings (which are mostly "imported" with netlib lapack I think).

@martin-frbg
Copy link
Collaborator

Hmm. Actually the travis logfile for the killed job is a mere 194k in size. I am beginning to wonder if we are seeing fallout from #1255 here, or if it was just a coincidental instability of the travis host ?

@isuruf
Copy link
Contributor Author

isuruf commented Aug 17, 2017

@brada4
Copy link
Contributor

brada4 commented Aug 17, 2017

Small clarification about limitations added, guide looks good.

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.

3 participants