RoaringBitmap / CRoaring

Roaring bitmaps in C (and C++), with SIMD (AVX2, AVX-512 and NEON) optimizations: used by Apache Doris, ClickHouse, and StarRocks
http://roaringbitmap.org/
Other
1.54k stars 265 forks source link

Compilation on windows? #46

Closed mrboojum closed 8 years ago

mrboojum commented 8 years ago

Are there any plans to support windows 64bit, preferably with the visual studio compiler or else mingw/cygwin?

I got it to compile and run on VS2015 (unit test still fail) but this already required quite some changes to the code.

lemire commented 8 years ago

@mrboojum

It is a C99-standard library while VS2015 front-end compiler is not standard compliant.

To solve this issue, Microsoft allows developers to use clang as a front-end. Because clang is standard compliant, it should support CRoaring just fine, though it is untested at this point. What happens if you use Clang with Microsoft CodeGen? It should solve your issue. If not, please report back.

Reference:

https://blogs.msdn.microsoft.com/vcblog/2015/12/04/clang-with-microsoft-codegen-in-vs-2015-update-1/

fsaintjacques commented 8 years ago

We also use C11 features which VS2015 is not supporting.

lemire commented 8 years ago

@fsaintjacques Right, but clang ought to support them... so Clang with Microsoft CodeGen should work.

Auralytical commented 8 years ago

The only issue I had compiling for Windows under mingw-w64 was the posix_memalign calls.

lemire commented 8 years ago

@RogueException Does this commit help?

https://github.com/RoaringBitmap/CRoaring/commit/100397dd815354953532e4c57ceed7eb5f2dbd0b

Do you have Clang with Microsoft CodeGen?

Auralytical commented 8 years ago

Still fails on mingw-w64 Builds successfully for x86 with CodeGen after including malloc.h x64 with CodeGen fails with '_mm_shuffle_epi8': Intrinsic not yet implemented

lemire commented 8 years ago

Still fails on mingw-w64

Surely we are very close. Do you possibly have a patch we could apply?

Builds successfully for x86 with CodeGen after including malloc.h

Where is the missing malloc.h?

(This probably builds without all the nifty Intel-specific optimizations so it is not so exciting.)

x64 with CodeGen fails with '_mm_shuffle_epi8': Intrinsic not yet implemented

They have not yet implemented the Intel intrinsics? That's not great.

Auralytical commented 8 years ago

I added malloc.h to the top of portability.h. Not sure if it should be controlled by any of the defines there?

lemire commented 8 years ago

Is adding this malloc enough to get a succesful build on mingw?

Auralytical commented 8 years ago

malloc is blocking CodeGen x86. #52 appears to be all we need for mingw x64.

lemire commented 8 years ago

malloc.h is a standard header and there is no harm to spraying it all over... so if you need to add it to get the code to build, can you issue a PR? It won't harm us.

lemire commented 8 years ago

Excellent.

mrboojum commented 8 years ago

Thanks for all the responses. I've installed clang in visual studio update 3. Please note that I've manually defined a solution with a static library and an executable for the tests. This because the cmake file "FindLTO.cmake" didn't contain anything for visual studio (line 14 # TODO).

However the compiler first crashes on the library compilation after creating some warnings like: 1>....\src\array_util.c(374,15): warning : always_inline function '_mm_lddqu_si128' requires target feature 'sse3', but would be inlined into function 'intersect_vector16' that is compiled without support for 'sse3'

I have tried adding various flags like: -march=native -msse4.2 but this results in: '_mm_shuffle_epi8': Intrinsic not yet implemented: %7 = call <16 x i8> @llvm.x86.ssse3.pshuf.b.128(<16 x i8> %4, <16 x i8> %6)

I'm a bit stuck here. Do you know the correct compilation flags or how the visual studio projects/solution can be created with cmake?

lemire commented 8 years ago

@mrboojum

As pointed out by @RogueException, it seems that even though Visual Studio allows you to compile with clang, it does not yet support Intel intrinsics. That's not great because, obviously, we make use of Intel intrinsics when we detect an x64 processor.

You might have to trick the code into thinking you do not have an x64 processor. See the portability.h file, where this detection is done.

It is also possible that Microsoft will complete support for clang in Visual Studio and include support for Intel intrinsics. That would be the best scenario. I am not sure why they chose not to support the Intel intrinsics... as it effectively cripples their support for clang... Maybe it is a temporary omission.

It would be great if Microsoft were to provide some kind of documentation on these matters.

mrboojum commented 8 years ago

Thanks for the reply, I understand the comment from @RoqueException now (this is my first encounter with "Intel intrinsics". I ques this leaves me with 4 options: 1) File a bug report at Microsoft and hope they will provide a workaround/fix. I'll do this but I don't expect a resolution soon 2) Compile croaring to dll with mingw x64 and then incorporate that in the existing visual studio compiled code. 3) Switch the whole application to mingw x64. 4) Trick the code into thinking I don't have an x64 processor as suggested. I did this by commenting out the inclusion of "" and instead include "". I ques that if this works this comes at a performance penalty. The library compiles now but building the tests fail due to the inclusion of strings.h in cmocka.c, which is not available on windows. I have tried to replace the cmocka files with the cmocka download (which doesn't include strings.h) but then some functions are missing. Could strings.h be removed/replaced?

Thanks for the help sofar, I really want to avoid scenario 2/3.

lemire commented 8 years ago

@mrboojum

Get the latest release (v0.2.5). Then try compiling first DISABLEAVX defined. If that does not work, try compiling with DISABLE_X64 defined. The latter should definitively work.

This code builds and run on a Raspberry Pi... so there should be no problem.

1) File a bug report at Microsoft and hope they will provide a workaround/fix. I'll do this but I don't expect a resolution so

I do recommend that you do this. We don't need a workaround though. We need a bona fide fix where Microsoft provides full support for Intel intrinsics through clang. This is essential if we are to have the best performance.

nkurz commented 8 years ago

We need a bona fide fix where Microsoft provides full support for Intel intrinsics through clang. This is essential if we are to have the best performance.

Well, there are other ways to get there. One approach is that we could distribute assembly for the core routines rather than using intrinsics. Another is that we could distribute binary (either of those core routines or of the entire library) rather than source.

The big disadvantage of both of these is that we don't get performance portability to processors that support the intrinsics but are not binary compatible. The big advantage is that once we find a high performance solution for a particular processor we can "lock it in" rather than being susceptible to compile time options and compiler version changes.

Arguably, the intrinsics approach isn't that portable to begin with, and using compile time flags to choose execution paths is a undebuggable combinatorial nightmare. Rather than scattering them throughout the source, it may be cleaner to clearly separate the portable C from a lower level of processor specific code.

lemire commented 8 years ago

One approach is that we could distribute assembly for the core routines rather than using intrinsics. Another is that we could distribute binary (either of those core routines or of the entire library) rather than source.

True. We could prepare and distribute pre-compiled and pre-tested binaries.

Because @RogueException has confirmed that it is possible to build CRoaring using mingw, then it is a simple matter of building the DLLs and posting them online...

Arguably, the intrinsics approach isn't that portable to begin with, and using compile time flags to choose execution paths is a undebuggable combinatorial nightmare.

True. My current testing protocol is outlined in this script which tests both an x64-optimized build an a generic one...

https://github.com/RoaringBitmap/CRoaring/blob/master/tools/prereleasetests.sh

(We used to have such a script but it somehow disappeared in the move to cmake so I recreated one. I don't think that the cmake approach to testing is sufficient. You want to build several configurations systematically and test them all. A bash script does the job well.)

I think it is reasonable.

Ezibenroc commented 8 years ago

This code builds and run on a Raspberry Pi... so there should be no problem.

Is it still true? I tried on my Raspberry Pi, it compiled successfully (with a lot of warnings), but the test mixed_container_unit failed. I use the model "2 type B", with Raspbian.

lemire commented 8 years ago

@Ezibenroc Let me see.

lemire commented 8 years ago

@Ezibenroc You are right, there is a test failure for mixed_container_unit on my raspberry pi. I wonder why.

lemire commented 8 years ago

All other tests pass however... so it is a bit mysterious. Let me see.

mrboojum commented 8 years ago

I have posted a bug with Microsoft. Please let me know if there is more information that should be added.

Defining DISABLE_X64 results in a neatly compiled lib, thanks for the suggestion.

Thanks for removing the inclusion in cmocka.c, However now building the test fail with the errors: "no snprintf compatible function found" and "No vsnprintf compatible function found" After checking that those functions are in stdio.h I added: #define HAVESNPRINTF 1 and #define HAVEVSNPRINTF 1 in the top of the cmocka_private.h. However this results in an internal compiler error. Do you have any suggestions on how to fix those errors? Would it be possible to download and build cmocka and link against this? (I downloaded a precompiled version of cmocka but that was build with visual studio 2013).

lemire commented 8 years ago

@Ezibenroc Ok, it does work on a Raspberry Pi... but it caught a tiny bug in the unit test : https://github.com/RoaringBitmap/CRoaring/commit/3464687ba747f23f9c8dff5e419c6cc79a64c1a8

The code of the library itself was fine on the Raspberry Pi.

It would be useful to setup continuous testing on some ARM platform...

lemire commented 8 years ago

@mrboojum

The vsnprintf is definitively part of the modern C standards. It is also supported generally by Microsoft Visual Studio 2015 as you can see: https://msdn.microsoft.com/en-us/library/1kt27hek.aspx

Here is an example found on MSDN

#include <stdio.h>
#include <stdarg.h> // for va_list, va_start
#include <string.h> // for memset

#define BUFFCOUNT (10)

void FormatOutput(char* formatstring, ...)
{
    int nSize = 0;
    char buff[BUFFCOUNT];
    memset(buff, 0, sizeof(buff));
    va_list args;
    va_start(args, formatstring);
    nSize = vsnprintf(buff, sizeof(buff), formatstring, args);
    printf("nSize: %d, buff: %s\n", nSize, buff);
}

int main() {
    FormatOutput("%s %s", "Hi", "there");   //  8 chars + null
    FormatOutput("%s %s", "Hi", "there!");  //  9 chars + null
    FormatOutput("%s %s", "Hi", "there!!"); // 10 chars + null
}

If it crashes your compiler you need to report the issue to Microsoft.

lemire commented 8 years ago

@mrboojum Note that you should make sure to specify that you require the C11 standard.

nkurz commented 8 years ago

@lemire I'm not sure if you're aware, but MSVC is famously not a C compiler at all and does not support (to my knowledge) any of the C standards. It's a C++ compiler that has some ad-hoc C extensions. I doubt there is a way to specify C11 compatibility, and if there is, it almost certainly doesn't actually support full C11 features. This said, you are right that vsnprintf() should be supported, since I think it's one of the parts of C99 that was incorporated into the C++11 standard: https://msdn.microsoft.com/en-us/library/hh409293.aspx

lemire commented 8 years ago

@nkurz I pulled out my Windows laptop with VS 2015 on it. Let me see.

lemire commented 8 years ago

@nkurz

The idea is that you do not compile with the usual Visual Studio C++ compiler, but instead your use clang as the front-end. That's how @mrboojum was about to actually compile CRoaring with Visual Studio even though it requires C11. My understanding is that he fails to compile the unit tests, no doubt due to minor configuration reasons.

I tried to see if I could reproduce his good results, but I only have Visual Studio Community Edition on my laptop and I don't think it supports clang CodeGen.

Auralytical commented 8 years ago

@lemire VS2015 Community Edition is nearly full featured, and will support CodeGen. Make sure you have the latest update for it (Update 3, I believe?).

lemire commented 8 years ago

@RogueException

I have the update 3. The meagre documentation I found says to do this:

This brings up nothing. I see that there is a separate download... Trying this now.

Auralytical commented 8 years ago

It's found under Cross Platform Mobile Development -> Visual C++ Mobile Development in the Modify features list

lemire commented 8 years ago

@RogueException I see this now.

nkurz commented 8 years ago

The idea is that you do not compile with the usual Visual Studio C++ compiler, but instead your use clang as the front-end.

I understand the idea, but not the implementation. I also recall that when Microsoft's Clang front-end approach was announced, I heaved a sign of relief and said "Finally, Microsoft gets a native C compiler and there's one less place I have to deal with C++". And then someone with greater knowledge of the implementation said "No, it's still a C++ compiler, just with a Clang front end".

I don't quite understand how this can be , but I'm still dubious that you will actually get C11 compatibility by using it. I think that most of @mrboojum's success may have come from the C99 features that C++11 incorporated, rather than from than direct C support. But I'd be quite happy to learn that this is not the case.

lemire commented 8 years ago

Ah Ah

So @nkurz is wrong. Once you have a clang CodeGen project, you can go under Configuration Properties, C/C++, Language, C Language Standard and choose between C89, C99, C11, C99 GNU, C11 GNU.

@mrboojum : what did you select for "C Language Standard"? The default, I think, would be C89 but we need to specify C11.

Sadly I am not familiar enough with Visual Studio to quickly turn CRoaring into a valid Visual Studio CodeGen project. If someone who is more at ease with Visual Studio 2015 (maybe @mrboojum or @RogueException) could give us a short tutorial on where to click to get it setup, I think it would be great.

Auralytical commented 8 years ago

@lemire I dont remember doing much more than adding the amalgamated .c and .h, and setting the platform to WIn32 (x86). I have an example project here: https://github.com/RogueException/CRoaring.Net/tree/master/src/CRoaring

EDIT: I never tried compiling the testbeds though.

lemire commented 8 years ago

@nkurz @RogueException

I'll see how far I can get tomorrow but the UI certainly does suggest that we get the full C standard.

mrboojum commented 8 years ago

Thanks for the support on this. My visual studio project files are here but those are using the original source files (and not the amalgamated.c and .h files).

It seems to me that failing to compile the test case with cmocka is indeed due to configuration reasons. Perhaps there are some other missing defines (like HAVE__SNPRINTF) that should have been added. The compiler exits in error somewhere else (perhaps not related to vsnprintf itself). See the error message below.

Next step would be incorporating the CRoaring library (compiled with clang) in a normal visual studio project. However this doesn't compile due to SIZEOF_LONG_LONG in portability.h not being defined. If I define this then the next error is a missing __builtin_popcountll definition (line 115 wih a comment that it won't work in visual studio:)) I ques I'm missing something else?

Error message for tests: C:\Program Files (x86)\Windows Kits\10\Include\10.0.10240.0\ucrt\stdio.h(205,24) : note: 'fopen' has been explicitly marked deprecated here 2> _ACRTIMP FILE* cdecl fopen( 2> ^ 2>....\tests\vendor\cmocka\cmocka.c(1814,18): warning : 'fopen' is deprecated: This function or variable may be unsafe. Consider using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [-Wdeprecated-declarations] 2> fp = fopen(buf, "w"); 2> ^ 2> C:\Program Files (x86)\Windows Kits\10\Include\10.0.10240.0\ucrt\stdio.h(205,24) : note: 'fopen' has been explicitly marked deprecated here 2> _ACRTIMP FILE* cdecl fopen( 2> ^ 2>....\tests\vendor\cmocka\cmocka.c(1946,23): warning : implicitly declaring library function 'strdup' with type 'char (const char )' [-Wimplicit-function-declaration] 2> msg = strdup(error_message); 2> ^ 2> ....\tests\vendor\cmocka\cmocka.c(1946,23) : note: include the header or explicitly provide a declaration for 'strdup' 2>d:\workspace_github\croaring\msvc\croaringtest....\tests\vendor\cmocka\cmocka.c(2618): fatal error C1001: An internal error has occurred in the compiler. 2> (compiler file 'c:\agent\build\cache\git\vctools\vctools\compiler\utc\src\p2\main.c', line 249) 2> To work around this problem, try simplifying or changing the program near the locations listed above. 2> Please choose the Technical Support command on the Visual C++ 2> Help menu, or open the Technical Support help file for more information 2> clang!DllGetC2Telemetry()+0x19f480

lemire commented 8 years ago

@mrboojum

Next step would be incorporating the CRoaring library in a normal visual studio project. However this doesn't compile

Visual Studio is a C++ compiler with no support for C standards whereas CRoaring is a standard-compliant C project. We could fix the first two problems you identified, but it is likely that they are the first two out of many others. Porting the library to Visual Studio C++ is beyond our scope at this time.

You can, however, create a header file for Visual Studio that would provide function signatures for your library. If someone provides us with such a Visual Studio C++ header file, we will be glad to incorporate it in the project.

lemire commented 8 years ago

@RogueException

You should now be able to compile for x64, but you should set DISABLE_X64 to disable our x64 optimizations.

Auralytical commented 8 years ago

I can confirm that it compiles for x64 with that flag, using VS's Clang

lemire commented 8 years ago

Thanks

I can also confirm that it compiles for x64, x86...

The trick (thanks: @RogueException ) is to disable intellisense since it is confusing you with made up non-errors. Evidently, intellisense can't parse standard compliant C code even when the compiler is happy with it. Thankfully, it is easy to disable the intellisense "errors": Tools -> Options -> Text Editor -> C/C++ -> Advanced, and set Disable Error Reporting to True.

Now we need someone to package this up in a nice DLL, a minimalist header file for DLL users, with actual testing and a tiny bit of documentation.

Note to @nkurz : it looks like we do get a true C compiler out of Visual Studio by choosing Clang with CodeGen (Cross Platform Mobile Development -> Visual C++ Mobile Development). So your pessimism was unwarranted.

nkurz commented 8 years ago

So your pessimism was unwarranted.

I'm glad to be wrong here.

Along with the recent Bash (https://msdn.microsoft.com/en-us/commandline/wsl/about) and Ubuntu (http://insights.ubuntu.com/2016/03/30/ubuntu-on-windows-the-ubuntu-userspace-for-windows-developers/) integration into Win10, this is an interesting convergence. Did you know that you can now run Ubuntu binaries directly on Win10? Like apt-get from the same repository and it just works?

This does remind me of one more immediate compilation alternative I forgot: Intel's compiler has full support both Windows and intrinsics. Compiling with ICC should work out of the box.

mrboojum commented 8 years ago

@lemire I don't quite understand your suggestion "create a header file for Visual Studio that would provide function signatures". How would the function signatures in this header differ from the ones in the cpp header that is already part of the project (roaring.hh)?

So to try this out I moved the implementation of all methods in roaring.hh to a roaring_cpp.cpp (with underscore _cpp because visual studio can't cope with two source files with the same name) together with the roaring.h inclusion. I replaced the roaring_bitmap_t pointer with a void pointer and casted that back in roaring_cpp.cpp (to prevent dependency on roaring.h). The library compiles now and the executable builds with linking to this lib. Please let me know if this roughly corresponds with your suggestion.

But the funny thing is that the test still crashes in roughly the same place as the library that I've build previously by changing the code (instead of using clang). The crash occurs after the invocation of fastunion in bitset_container_free. Perhaps somehow the memory got allocated with new (but I don't see where)?

lemire commented 8 years ago

@mrboojum

The library compiles now and the executable builds with linking to this lib. Please let me know if this roughly corresponds with your suggestion.

Something like that yes.

CRoaring is a C library. Visual Studio provides a C++ compiler. So it is probably best to build a library that offers a C++ interface.

But the funny thing is that the test still crashes in roughly the same place as the library

Ok. Here is what I suggest you do.

First create a GitHub repository for the DLL. In this repository, copy the unity-build files. For your convenience, you can find them there...

https://github.com/lemire/CRoaringUnityBuild

Check in your project file and include all necessary documentation so that we can build the library.

Then publish another Visual Studio solution (still on GitHub), where you run the unit tests on top of the newly generated library.

If you publish your work, we should be able to figure it out.

Otherwise, it is hopeless to debug whatever might have happened on your machine.

lemire commented 8 years ago

@mrboojum And ideally, take the output of the unity build (roaring.h and roaring.c) "as is": do not modify them. If you do, then upgrading your library will become difficult.

mrboojum commented 8 years ago

Thanks for the suggestion. I have created a githhub repository (called CRoaring4VS) containing the library as you suggested. It's here. The repository contains 1 solution with 2 projects: CRoaring4VSLib to build the library and CRoaring4VSTest to test the library. The provided source files are not edited (the define DISABLE_X64 is now in the prject setttings). The instructions are in the readme. Please let me know if there is anything incorrect/missing in this setup.

During the construction of this repository I noticed that the error message in the 64bit debug build contains somewhat more information, this reports "The Block at 0x.... was allocated by aligned routines, use _aligned_free()." I hope this helps.

lemire commented 8 years ago

@mrboojum At first glance, this looks very nice.

lemire commented 8 years ago

@mrboojum Yes, you did nice work. It was easy for me to fix the problem on my box... please confirm the fix: https://github.com/mrboojum/CRoaring4VS/pull/1

lemire commented 8 years ago

I consider that the issue is resolved with https://github.com/mrboojum/CRoaring4VS

If there are specific issues, we should address them specifically.