-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Feature Request: Add Cygwin support #156
Comments
My fork https://github.com/zmajeedforks/oneTBB/tree/cygwin_build is based on latest master - it has changes for building on cygwin - master...zmajeedforks:cygwin_build - I'll give you a PR when tests pass - and update my progress here |
First I got these cmake errors
I didn't know what tbbbind or hwloc is - found it's related to MPI which I'm not interested in though seems supported on cygwin - could look into later - it seems optional and unnecessary to see something that feels like nothing might build I decided to ignore it and proceeded with building |
Next
Turns out |
Very happy to see |
btw I'm building with gcc 11.2 and no problems so far |
then - need |
then - #pragma weak __TBB_make_rml_server but weak symbols have some subtleties on cygwin - I don't yet know why weak symbols are needed at all - so disabled them in #define __TBB_WEAK_SYMBOLS_PRESENT ( !_WIN32 && !__APPLE__ && !__sun && !__CYGWIN__ && (__TBB_GCC_VERSION >= 40000 || __INTEL_COMPILER ) ) |
next - reusing the linux proxy def file for cygwin - in set(TBB_DEF_FILE_PREFIX lin${TBB_ARCH}) |
cygwin has no exclude cygwin for the same reason as windows // On Windows there is no real thread number limit beside available memory.
// Therefore, the test for thread limit is unreasonable.
#if TBB_USE_EXCEPTIONS && !_WIN32 && !__ANDROID__ && !__CYGWIN__ |
finally - a number of test files have assembler errors - either this is due to heavy use of templates - arguably this is a gcc issue but simplifying templates could be worthwhile only the problem with these options is they increase compile times for files that don't need them - so I did not make instead the option is added to only those files that broke in # these files need -O3 to fix assembler errors for too many sections, file too big
if (CYGWIN)
set_source_files_properties(
tbb/test_concurrent_unordered_map.cpp
conformance/conformance_concurrent_unordered_map.cpp
...
PROPERTIES COMPILE_OPTIONS -O3
)
endif() |
so build works on cygwin - have not run tests yet |
5 tests failed for first build - all due to dynamic linking - cygwin has dlls not shared objects - I hoped cmake would just do the right thing but looks like it needs the tweaking I did for the older makefiles build
Errors
|
I think you need to update tests (perhaps, the library as well) that they use |
Can you please specify which tests cause the issue? |
My original changes to the old makefiles build included cygwin dll support - I'll make similar changes for the cmake build The note above about assembler errors includes a snippet from |
I've gone through the 5 test failures - I'll note what's happening with each below - couple of these are hairy and need a fair amount of debugging to sort out |
Here's how to run all tests
and run a single test,
and build a single test
|
The easiest was
|
Next is
Make the change
Rebuild and rerun test
|
The third test to fail is Luckily cygwin can use the Windows version of the test - this works! But it is not straightforward to get cygwin to use Windows API the way TBB tests are currently structured The first big hurdle is the plethora of platform feature test macros in the common header files for the tests. For cygwin to use Windows API, it has to A second problem is the common functions used in tests are all implemented with various platform-specific blocks in header files. It would be nice if common functions were in a separate library with implementations separated from the interfaces. Since this is not the case, refactoring is trickier. The third problem is the shared headers have hard dependencies on the doctest unit test framework used by TBB. doctest is a single header library. This means it cannot be built into a separate library and used by a test executable at the same time because of multiple definition errors for doctest symbols when linking Again it would be nice if shared functions that are not actually tests themselves did not depend on doctest - and the only dependencies were in the unit test .cpp files My temporary fix to get the test to pass is to place a copy of the shared function in Here's the original failure
The test passes after moving some code around in It's very slow though which could be cygwin overhead of using Win32
|
The last two tests to fail Both tests crash with segv faults that trash the stack so pinpointing the culprit is tedious The segv in both tests is from a dynamically loaded function from a dll in one thread that is accessed after the library has been unloaded with I'm trying to reproduce the failures in small standalone programs to see if the bugs are in TBB tests or in the doctest framework or in cygwin |
The description above for the fifth test failure The actual failure is two-fold - fixed by using a right library name and version - and by using the Win32 version of a utility function Initially the test fails because of a shared library/dll name and version mismatch. The top cmakefile has
This is fixed by correctly setting various macros for cygwin in
With the correct library loading, a test assertion fails
As it happens this same function Back to Lo and behold the test passes
|
Only one test failure remains - I'm thinking a destructor or some kind of cleanup code runs at program exit and not when the library is unloaded. This would be a bug in the test. |
Question for TBB maintainers - would TBBBind be required for a Cygwin port PR? |
Sync to upstream master added interprocedural optimization options by default - introduced in PR #432 These gcc options
Disabling IPO for cygwin in top cmakefile
It could be |
We do not use Cygwin :) so it is up to you. TBBBind is responsible for NUMA support in |
That's great - now I just need to fix this last test failure and come up with a nicer way for cygwin to use Win32 code in the tests |
Can it be related: https://github.com/oneapi-src/oneTBB/blob/master/test/common/doctest.h#L470-L504? |
Thank you for the tip - enabling the VC++ version of |
Well -
// list of all active pools is used to release
// all TLS data on thread termination or library unload this is not happening in
Why does this test work on other platforms? The Any help from TBB devs would be appreciated |
Found else if (callReason==DLL_PROCESS_DETACH)
{
__TBB_mallocProcessShutdownNotification(lpvReserved != NULL);
} This is exactly what's needed for cygwin - Enabling it for cygwin opens a small can of worms because this Also memory pool cleanup in #if __TBB_SOURCE_DIRECTLY_INCLUDED
/* Pthread keys must be deleted as soon as possible to not call key dtor
on thread termination when then the tbbmalloc code can be already unloaded.
*/
defaultMemPool->destroy();
destroyBackRefMain(&defaultMemPool->extMemPool.backend);
ThreadId::destroy(); // Delete key for thread id
hugePages.reset();
// new total malloc initialization is possible after this point
mallocInitialized.store(0, std::memory_order_release);
#endif // __TBB_SOURCE_DIRECTLY_INCLUDED The purpose of /** Internal TBB features & modes **/
/** __TBB_SOURCE_DIRECTLY_INCLUDED is a mode used in whitebox testing when
it's necessary to test internal functions not exported from TBB DLLs
**/ Now So it's not as simple as adding Anyway all tests now pass for cygwin - all that remains for a PR are cleaner ways for cygwin to sometimes use Win32 code
|
|
Thanks for clearing up the intended use of I found the true reason why pthread key destructors were crashing after a closer look at The name of the tbbmalloc library built by cmake on cygwin is RegisterProcessShutdownNotification() {
// prevents unloading, POSIX case
// We need better support for the library pinning
// when dlopen can't find TBBmalloc library.
// for example: void *ret = dlopen(MALLOCLIB_NAME, RTLD_NOW);
// MALLOC_ASSERT(ret, "Allocator can't load itself.");
dlopen(MALLOCLIB_NAME, RTLD_NOW);
} My initial changes for cygwin missed adding the version to With |
There is a working cygwin port at https://github.com/zmajeedforks/oneTBB/tree/cygwin_build |
Great! Do you have to contribute the port? |
I plan to - only one item needs cleaning up for a PR In a couple tests I had to make a copy of the Windows implementation of I want to get rid of the copy in the PR - the original So I tried placing I'll try again - any suggestions are also welcome |
Success! I refactored The refactoring turns My first attempt failed with link errors for multiple definitions of symbols from the doctest unit testing framework. Couple examples (from dozens) were
The reason is doctest is a single-header library. All functions are defined in Once the tests passed, I revisited the linker errors in order to get a refactored It turns out doctest deliberately eschews inline functions in the header in favor of configuration macros for applications to use doctest in a library. These macros are documented at https://github.com/onqtam/doctest/blob/master/doc/markdown/configuration.md. These macros include In particular, #if !defined(DOCTEST_CONFIG_IMPLEMENT)
#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#endif This led me to the solution, which is to make the define of #if !defined(DOCTEST_CONFIG_IMPLEMENT) && !TBB_TEST_NO_DOCTEST_CONFIG
#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#endif This way existing test code remains unaffected - and the new This works out nicely. There are 9 tests that depend on |
Can you please summarize the problem scope with cygwin and Win32 API to document design decisions? As you mentioned,
It does not seem that we need to solve the problem in a general way (at first glance a separate module and related issues seem overcomplicated). Is it possible to introduce the cygwin-related macro, e.g. |
@alexey-katranov is this still relevant? |
This issue is to track support for Cygwin - the Linux environment on Windows
I've added makefiles support to for a cygwin build that I'd like to send as a PR - should it be for the tbb_2019 branch or would you like to create a new tbb_2019_cygwin branch?
My changes are at tbb_2019...zmajeed:cygwin_build
Many tests still fail - I hope others will be able to contribute fixes to get the cygwin build fully working
The text was updated successfully, but these errors were encountered: