Support for Clang-CL on Windows

[RoyiAvital]

While there is some Windows Support it is limited to the GENERIC code path.

I suggest that support for Clang-CL will be added in CMAKELists.txt so if the compiler is Clang-CL things will work like GCC on Linux as Clang-CL should have support for AT&T style of assembly.

The problem is the configuration somehow doesn't support that.
I get errors like:

..\kernel\generic\kernel_dgemm_4x4_lib4.c(3056,15): error: expected ';' at end of declaration
        double CC[16] __declspec(align(64)) = {0};
                     ^
                     ;
..\kernel\generic\kernel_dgemm_4x4_lib4.c(3056,38): error: expected identifier or '('
        double CC[16] __declspec(align(64)) = {0};
                                            ^
..\kernel\generic\kernel_dgemm_4x4_lib4.c(6398,15): error: expected ';' at end of declaration
        double CC[16] __declspec(align(64)) = {0};
                     ^
                     ;
..\kernel\generic\kernel_dgemm_4x4_lib4.c(6398,38): error: expected identifier or '('
        double CC[16] __declspec(align(64)) = {0};
                                            ^
..\kernel\generic\kernel_dgemm_4x4_lib4.c(6480,15): error: expected ';' at end of declaration
        double CC[16] __declspec(align(64)) = {0};
                     ^
                     ;
..\kernel\generic\kernel_dgemm_4x4_lib4.c(6480,38): error: expected identifier or '('
        double CC[16] __declspec(align(64)) = {0};
                                            ^
..\kernel\generic\kernel_dgemm_4x4_lib4.c(6654,15): error: expected ';' at end of declaration
        double CC[16] __declspec(align(64)) = {0};
                     ^
                     ;
..\kernel\generic\kernel_dgemm_4x4_lib4.c(6654,38): error: expected identifier or '('
        double CC[16] __declspec(align(64)) = {0};
                                            ^
..\kernel\generic\kernel_dgemm_4x4_lib4.c(6757,15): error: expected ';' at end of declaration
        double CC[16] __declspec(align(64)) = {0};

Also it tries to use flags like -fPIC -msse3 etc...
Those flags matches GCC.

SO I suggest that the configuration will support Clang-CL as it was MSVC with the only different it will allow it to use different configurations.

I think the flags:

set(C_FLAGS_TARGET_X64_INTEL_HASWELL      "-m64 -mavx -mavx2 -mfma")
set(C_FLAGS_TARGET_X64_INTEL_SANDY_BRIDGE "-m64 -mavx")
set(C_FLAGS_TARGET_X64_INTEL_CORE         "-m64 -msse3")
set(C_FLAGS_TARGET_X64_AMD_BULLDOZER      "-m64 -mavx -mfma")

Needs to be decorated with -XClang or adapted to Windows.

[RoyiAvital]

I am not sure why, but it seems when the compiler is Clang-CL which defines both _MSC_VER and the GCC macros something bad happens.

The way I define portable aligned array is:

// SSE - 16 Byte Alignment, AVX - 32 Byte Alignment
// Example: DECLARE_ALIGN_PRE int array[5] DECLARE_ALIGN_POST = { 0, 1, 2, 3, 4 };
#if defined(USING_INTEL_COMPILER) || defined(_GCC_) || defined(__clang__)
#define DECLARE_ALIGN_PRE 
#define DECLARE_ALIGN_POST __attribute__((aligned(SIMD_ALIGNMENT)))
#elif defined(_MSC_VER)
#define DECLARE_ALIGN_PRE __declspec(align(SIMD_ALIGNMENT))
#define DECLARE_ALIGN_POST
#else
#define DECLARE_ALIGN_PRE 
#define DECLARE_ALIGN_POST
#endif // defined(USING_INTEL_COMPILER) || defined(_GCC_) || defined(__clang__)

It might reduce few lines in your code.

Do you have any idea what would trigger the issue with Clang?

[giaf]

I see, Clang-CL wants gcc alignment, so your code above works because GCC is defined, and it never gets into the MSC_VER.

My code is the other way around, so it would get into the MSC_VER.

[RoyiAvital]

By the way, if I use the GENERIC target it works and compiles with Clang-CL.
Tests are working, it is just this file (kernel_dgemm_4x4_lib4.c) which has some issues (In case of the HASWELL target 30 files before it compiled successfully).

[RoyiAvital]

I think Clang-CL supports both kind of alignments. I posted my method so you will be able to use one liner in your code instead of the long ifdef().

Could it be that the code has some issues in case both _MSC_VER and _GCC_ are defined?

[RoyiAvital]

Even with 29329ec I get errors building anything but the GENERIC target.
Those are the errors I get:

..\kernel\avx2/kernel_dgetrf_pivot_lib.c(2119,29): error: expected ';' at end of declaration
        double pU0[3*4*K_MAX_STACK] __declspec(align(64));
                                   ^
                                   ;
..\kernel\avx2/kernel_dgetrf_pivot_lib.c(2119,30): warning: declaration does not declare anything [-Wmissing-declarations]
        double pU0[3*4*K_MAX_STACK] __declspec(align(64));
..\kernel\avx2/kernel_dgetrf_pivot_lib.c(3403,29): error: expected ';' at end of declaration
        double pU0[3*4*K_MAX_STACK] __declspec(align(64));
                                   ^
                                   ;
..\kernel\avx2/kernel_dgetrf_pivot_lib.c(3403,30): warning: declaration does not declare anything [-Wmissing-declarations]
        double pU0[3*4*K_MAX_STACK] __declspec(align(64));

There also many warnings about unused variables.

Since GENERIC does compile I tend to think there is an issue with typing there indeed.

[RoyiAvital]

OK, I think I got it. You misplaced the __declspec(align(64)).

Instead of double pU0[3*4*K_MAX_STACK] __declspec(align(64)); it should be __declspec(align(64)) double pU0[3*4*K_MAX_STACK];

I really think you should take my form of teh code and put it in the h file and that's it.

[RoyiAvital]

I can confirm that indeed what I wrote in #120 (comment) is the error. If I switched them as I wrote compilation is successful.

Small fix in CMAKElists.txt:

# common C flags
if(CMAKE_C_COMPILER_ID MATCHES "GNU" OR CMAKE_C_COMPILER_ID MATCHES "Clang")
	# set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O2 -fPIC") # Original
	set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O2") # Royi
	if(NOT CMAKE_C_SIMULATE_ID MATCHES "MSVC") # Royi See https://stackoverflow.com/questions/49480535
		set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fPIC") # Royi
	endif() # Royi
endif()

[RoyiAvital]

I found a way using RegEx to correct this on all files - https://regex101.com/r/xzUcAt/8.

Do that and the little fix in #120 (comment) and I think we'll have a version which can, on Windows, have any target using Clang-CL. Nice isn't it?

[giaf]

Ok thanks for investigating and finding the issues!

I will fix stuff now.
It's not a big issue to change that in all files using vim, it is a very powerful editor for this kind of repetitive tasks :p

[RoyiAvital]

Please. Wait a second, I will send you a pull. I am just finishing it.

[RoyiAvital]

Done, Sent a pull request which enables full Windows support.

[giaf]

Ok so now with this commit b2b1377 alignment should be managed in a portable way using a macro.

@RoyiAvital could you check that this still works for you, then I can merge this in master.

[RoyiAvital]

@giaf , Sorry, but it doesn't work:

..\kernel\generic\kernel_dgemm_4x4_lib4.c(2185,2): warning: implicit declaration of function 'ALIGN' is invalid in C99 [-Wimplicit-function-declaration]
        ALIGN( double CC[16], 64 ) = {0};
        ^
..\kernel\generic\kernel_dgemm_4x4_lib4.c(2185,9): error: expected expression
        ALIGN( double CC[16], 64 ) = {0};
               ^
..\kernel\generic\kernel_dgemm_4x4_lib4.c(2185,31): error: expected expression
        ALIGN( double CC[16], 64 ) = {0};

Why won't you just copy the Macro I gave you?
It is tested to work.

[RoyiAvital]

By the way, it didn't work with MinGW64 as well:

C:\Users\User\BLASFeo\kernel\generic\kernel_dgemm_4x4_lib.c:21627:31: error: expected expression before '{' token
  ALIGN( double CC[16], 64 ) = {0};
                               ^
C:\Users\User\BLASFeo\kernel\generic\kernel_dgemm_4x4_lib.c:21633:72: error: 'CC' undeclared (first use in this function); did you mean 'C'?
  kernel_dgemm_nn_4x4_lib4ccc(kmax, &alpha1, A, B, ldb, &beta1, C, ldc, CC, bs);
                                                                        ^~
                                                                        C
C:\Users\User\BLASFeo\kernel\generic\kernel_dgemm_4x4_lib.c: In function 'kernel_dgetrf_nn_4x4_vs_lib4ccc':
C:\Users\User\BLASFeo\kernel\generic\kernel_dgemm_4x4_lib.c:21723:9: error: expected expression before 'double'
  ALIGN( double CC[16], 64 ) = {0};
         ^~~~~~
C:\Users\User\BLASFeo\kernel\generic\kernel_dgemm_4x4_lib.c:21723:31: error: expected expression before '{' token
  ALIGN( double CC[16], 64 ) = {0};
                               ^
C:\Users\User\BLASFeo\kernel\generic\kernel_dgemm_4x4_lib.c:21729:72: error: 'CC' undeclared (first use in this function); did you mean 'C'?
  kernel_dgemm_nn_4x4_lib4ccc(kmax, &alpha1, A, B, ldb, &beta1, C, ldc, CC, bs);
                                                                        ^~
                                                                        C

So I think the Macro you defined is wrong is some way.
Could it be it requires C99 or some other standard?

Update

I tried with C99 and C11 flags, both fail as well.

[giaf]

Yes sorry I made a bit of mess, first I went for ALIGN and then finally ALIGNED, but I forgot some apparently. Strange that it compiled for me. I'll fix it now.

About your macro, I didn't get what was the need for two macros (pre and post), while one seems to be enough. If one works, I would prefer it. Apart from that, the checks for the compiler are identical to yours.

[RoyiAvital]

Where is the definition of your macro?

[giaf]

Here it is https://github.com/giaf/blasfeo/blob/windows_support/include/blasfeo_common.h#L51

I just pushed the fixes ALIGN => ALIGNED. If this was the only issue, it should work now.

[RoyiAvital]

OK, I see. It is nice.

By the way:

if defined(__INTEL_COMPILER) || defined(__ICL) || defined(__ICC) || defined(__INTEL_LLVM_COMPILER)
#define USING_INTEL_COMPILER

Just to make it correct. I didn't put it before. My mistake.

Tell me where I can download the push with the fixes (What I posted now included) and I will test it.

[giaf]

There was another issue, apparently the correct macro for gcc is __GNUC__ instead of _GCC_.
I fixed also that, the code is here
https://github.com/giaf/blasfeo/tree/windows_support

[RoyiAvital]

OK,
Build seems to work.

@giaf , could you add in CMakeLists.txt the following:

project(blasfeo C ASM)

if(${CMAKE_C_COMPILER_ID} MATCHES "MSVC" OR CMAKE_C_SIMULATE_ID MATCHES "MSVC")
	if(${USE_STATIC_MSVCR})
		string(REPLACE "/MD" "/MT" CMAKE_C_FLAGS_DEBUG ${CMAKE_C_FLAGS_DEBUG})
		string(REPLACE "/MD" "/MT" CMAKE_C_FLAGS_RELWITHDEBINFO ${CMAKE_C_FLAGS_RELWITHDEBINFO})
		string(REPLACE "/MD" "/MT" CMAKE_C_FLAGS_RELEASE ${CMAKE_C_FLAGS_RELEASE})
		string(REPLACE "/MD" "/MT" CMAKE_C_FLAGS_MINSIZEREL ${CMAKE_C_FLAGS_MINSIZEREL})
	endif()
endif()

It will allow Windows users to chose the MSVCR version using simple variable.
We need this because CMake 3.15 and won't allow the user do it using the regular command line flags. Since some users build there project with /MT and the default is /MD it means compilation will fail on their system because BLASFEO won't match their project.

Once you add this, I will update the Windows Compilation Guide accordingly.

By the way, can we remove the libm dependency on the tests?