Page MenuHomePhabricator

Support clang compiling under windows-gnu and windows-msvc
ClosedPublic

Authored by SquallATF on Oct 19 2018, 2:08 AM.

Details

Summary

compiling under mingw still need masm because z_Windows_NT-586_asm.asm need masm to build, but can use uasm replace masm.

I have build both x86_64 and i686 versions under mingw without OMPT, and pass all tests.
i686 version have 2 tests need run with administrator privileges.

clang-cl can build, but can not pass tests, because most tests not support clang-cl.

Diff Detail

Repository
rL LLVM

Event Timeline

SquallATF created this revision.Oct 19 2018, 2:08 AM
SquallATF retitled this revision from Support mingw-w64 build with clang to Support clang compiling under windows.Oct 21 2018, 2:08 AM
SquallATF edited the summary of this revision. (Show Details)

I don't know whom to add as reviewer here unfortunately. Most of the changes look pretty straightforward, but there's quite a lot of them. Would it make sense to split some of it into separate reviews/commits, to allow more explanations for each kind of change?

SquallATF updated this revision to Diff 170337.Oct 21 2018, 7:35 AM

Support clang compiling under windows-gnu and windows-msvc

SquallATF retitled this revision from Support clang compiling under windows to Support clang compiling under windows-gnu and windows-msvc.
SquallATF updated this revision to Diff 172100.Nov 1 2018, 2:41 AM
SquallATF retitled this revision from Support clang compiling under windows-gnu and windows-msvc to Support clang compiling under windows-gnu and windows-msvc.
  • misc fix, forget commit -mrtm flag handle.
jlpeyton requested changes to this revision.Nov 2 2018, 3:34 PM

Add two lines in kmp_config.h.cmake:

#cmakedefine01 MSVC
#define KMP_MSVC_COMPAT MSVC

And instead of having the !defined(__MINGW32__) or #ifndef __MINGW32__ guard, use KMP_MSVC_COMPAT instead.

runtime/cmake/config-ix.cmake
85–87 ↗(On Diff #172100)

I don't think the if(CMAKE_C_COMPILER_ID STREQUAL "Clang") condition is necessary here.

runtime/src/kmp_affinity.h
378–397 ↗(On Diff #172100)

Change all size_t i to int i instead of adding the static_cast<size_t>

runtime/src/kmp_dispatch.cpp
27 ↗(On Diff #172100)

It would be nice if you could add another macro to kmp_os.h like this:

#define KMP_USE_X87CONTROL (KMP_OS_WINDOWS && KMP_ARCH_X86 && !KMP_COMPILER_CLANG)

and use it here and below in this file.

What was the problem with clang-cl? Why did it fail with _control87()?

runtime/src/kmp_os.h
249 ↗(On Diff #172100)

This use of __MINGW32__ is ok.

299 ↗(On Diff #172100)

Will using KMP_MSVC_COMPAT work here as well instead of __GNUC__?

runtime/src/kmp_settings.cpp
5516–5520 ↗(On Diff #172100)

Change num_bits_in_mask from int to DWORD instead of static_cast<>

This revision now requires changes to proceed.Nov 2 2018, 3:34 PM
SquallATF updated this revision to Diff 172577.Nov 5 2018, 6:06 AM
  • update patch
jlpeyton accepted this revision.Dec 7 2018, 2:28 PM

Sorry for late reply, this looks good to me.

This revision is now accepted and ready to land.Dec 7 2018, 2:28 PM

@jlpeyton Please help me commit this patch and depend, I did not have permission to commit it

This revision was automatically updated to reflect the committed changes.