WIP: Fix MSVC build of threading code #13970

tkelman · 2015-11-13T10:08:47Z

Not sure if this code actually works on Windows (LLVM 3.7.0 crashes at the start of bootstrap with a Relocation type not implemented yet! UNREACHABLE executed at /home/Tony/julia/deps/srccache/llvm-3.7.0/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:230!), but at least we can check whether it builds. Some of the !defined(_OS_WINDOWS_) in here should maybe actually be !defined(_COMPILER_MICROSOFT_), but we can sort that out when newer LLVM and the threading code are closer to being enabled by default.

tkelman · 2015-11-13T10:31:33Z

Ah, yeah, with mingw we get

In file included from /home/Tony/julia32/src/builtins.c:24:0:
/home/Tony/julia32/src/julia.h:592:1: warning: ‘thread’ attribute directive ignored [-Wattributes]
 extern JL_THREAD_EXPORT jl_gcframe_t *jl_pgcstack;
 ^

so JL_THREAD should definitely be gcc __thread there rather than __declspec(thread). Unfortunately, we get

ERROR: could not load symbol "jl_pgcstack":
The specified procedure could not be found.

It's not possible to export a thread-local variable from a dll: https://sourceforge.net/p/mingw-w64/mailman/message/31777672/

So the approach for dealing with jl_pgcstack will have to change if we ever want threading support on Windows.

yuyichao · 2015-11-13T12:45:38Z

What does mingw support for exporting TLS? pthread_key_create? Does thread_local work in c++11 mode?

tkelman · 2015-11-13T13:06:18Z

pthread_key_create would add a dependency on libwinpthread-1.dll, but that may not be the end of the world. We usually use the win32-threads variant of mingw-w64, but I believe that has a libwinpthread that we could try using. Regarding thread_local in c++11 mode, I know some of the c++11 thread constructs require the pthreads variant of mingw-w64, but I'm not sure whether thread_local is one of those. If you have any suggested patches I'm happy to test.

vtjnash · 2015-11-13T18:53:53Z

So the approach for dealing with jl_pgcstack will have to change if we ever want threading support on Windows.

Curious. I think that means that if we want TLS support everywhere, we would need to simulate it. That could be annoying, but it might give better efficiency than we have now, and avoid the need to use a modified version of llvm...

yuyichao · 2015-11-13T18:56:51Z

but it might give better efficiency than we have now

What do you mean by better efficiency? I think gcc support a emulate tls mode (not sure which ABI) that is much less efficient than the native TLS on windows.

tkelman · 2015-11-13T19:05:58Z

I should say I was using unpatched LLVM 3.7.0 here because I have a copy of that built, I have not tried with the tls branch yet. You have said elsewhere that it does not include a windows implementation of the relevant llvm features though?

vtjnash · 2015-11-13T19:09:02Z

general TLS support is expensive, but I think we could potentially choose to emulate only the parts that can be done efficiently. there are a few other trades that we could potentially do as well. it would still be slower than the current global variable, but I could see it providing some extra flexibility, speed, and/or reliability for certain operations.

vtjnash · 2015-11-15T00:43:21Z

src/threading.c

-    jl_all_task_states = malloc(jl_n_threads * sizeof(jl_thread_task_state_t));
+    jl_all_heaps =
+#ifdef __cplusplus
+        // undeclared?


you need to either go with struct _jl_thread_heap_t or jl_thread_heap_t, not half way in-between.

vtjnash · 2015-11-19T17:23:36Z

src/julia.h

@@ -87,7 +92,7 @@ DLLEXPORT void jl_threading_profile(void);
 #  define JL_ATOMIC_FETCH_AND_ADD(a,b)                                    \
       _InterlockedExchangeAdd((volatile LONG *)&(a), (b))
 #  define JL_ATOMIC_COMPARE_AND_SWAP(a,b,c)                               \
-       _InterlockedCompareExchange64(&(a), (c), (b))
+       _InterlockedCompareExchange64((volatile LONG64 *)&(a), (c), (b))


i would prefer to avoid these casts. can we fix the declarations of a to not need them?

no idea. FETCH_AND_ADD as well?

looks like I can get rid of the error and cast here for at least COMPARE_AND_SWAP by changing JL_DEFINE_MUTEX (and JL_DEFINE_MUTEX_EXT) to use int64_t volatile m instead of uint64_t volatile m. Is that okay on unix?

that ends up causing warnings

/src/julia.h:115:39: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] if (m ## _mutex == uv_thread_self())

so pick your poison I guess?

i guess i would slightly rather have the cast on the comparison (casting the lhs to a uv_thread_t) rather than the pointer. the failure behavior is a bit more localized in the event the definition changes.

that doesn't work, uv_thread_t is a HANDLE on windows

d:/code/msys64/home/Tony/julia/src/jltypes.c(1910) : error C2446: '==' : no conv ersion from 'unsigned long' to 'uv_thread_t' Conversion from integral type to pointer type requires reinterpret_cast, C-style cast or function-style cast d:/code/msys64/home/Tony/julia/src/jltypes.c(1910) : error C2040: '==' : 'uv_thr ead_t' differs in levels of indirection from 'unsigned long'

Use gcc __thread when building with gcc on windows DLLEXPORT JL_THREAD except on MSVC should help with: d:\code\msys64\home\tony\julia\src\julia.h(591) : error C2492: 'jl_pgcstack' : ' thread' data may not have dll interface d:\code\msys64\home\tony\julia\src\julia.h(1485) : error C2492: 'jl_current_task ' : 'thread' data may not have dll interface d:\code\msys64\home\tony\julia\src\julia.h(1486) : error C2492: 'jl_root_task' : 'thread' data may not have dll interface d:\code\msys64\home\tony\julia\src\julia.h(1487) : error C2492: 'jl_exception_in _transit' : 'thread' data may not have dll interface d:\code\msys64\home\tony\julia\src\julia.h(1488) : error C2492: 'jl_task_arg_in_ transit' : 'thread' data may not have dll interface d:/code/msys64/home/Tony/julia/src/jltypes.c(1910) : error C2664: 'LONG64 _Inter lockedCompareExchange64(volatile LONG64 *,LONG64,LONG64)' : cannot convert argum ent 1 from 'volatile uint64_t *' to 'volatile LONG64 *' Types pointed to are unrelated; conversion requires reinterpret_cast, C- style cast or function-style cast d:/code/msys64/home/Tony/julia/src/jltypes.c(1913) : error C2664: 'LONG64 _Inter lockedCompareExchange64(volatile LONG64 *,LONG64,LONG64)' : cannot convert argum ent 1 from 'volatile uint64_t *' to 'volatile LONG64 *' Types pointed to are unrelated; conversion requires reinterpret_cast, C- style cast or function-style cast d:/code/msys64/home/Tony/julia/src/jltypes.c(1993) : error C2664: 'LONG64 _Inter lockedCompareExchange64(volatile LONG64 *,LONG64,LONG64)' : cannot convert argum ent 1 from 'volatile uint64_t *' to 'volatile LONG64 *' Types pointed to are unrelated; conversion requires reinterpret_cast, C- style cast or function-style cast d:/code/msys64/home/Tony/julia/src/jltypes.c(2000) : error C2664: 'LONG64 _Inter lockedCompareExchange64(volatile LONG64 *,LONG64,LONG64)' : cannot convert argum ent 1 from 'volatile uint64_t *' to 'volatile LONG64 *' Types pointed to are unrelated; conversion requires reinterpret_cast, C- style cast or function-style cast

to move casts from JL_ATOMIC_COMPARE_AND_SWAP to JL_LOCK/JL_UNLOCK

these don't seem to be needed

tkelman · 2015-11-21T19:32:17Z

This is mostly made redundant by #14083. Will move the remaining bit over there once I get a chance to build it on msvc.

tkelman added system:windows Affects only Windows multithreading Base.Threads and related functionality labels Nov 13, 2015

tkelman mentioned this pull request Nov 13, 2015

Windows build with JULIA_THREADS = 1 segfaults #13975

Closed

vtjnash reviewed Nov 15, 2015
View reviewed changes

tkelman force-pushed the tk/msvc-threads branch 2 times, most recently from 5ca56b6 to d6dbf88 Compare November 19, 2015 02:04

vtjnash reviewed Nov 19, 2015
View reviewed changes

tkelman force-pushed the tk/msvc-threads branch from d6dbf88 to 37a0517 Compare November 19, 2015 20:04

tkelman added 3 commits November 20, 2015 14:20

Use int64_t instead of uint64_t in JL_DEFINE_MUTEX

d9d3b7f

to move casts from JL_ATOMIC_COMPARE_AND_SWAP to JL_LOCK/JL_UNLOCK

Do not dllexport jl_root_task or jl_task_arg_in_transit

46b8bf9

these don't seem to be needed

tkelman force-pushed the tk/msvc-threads branch from 0a6663c to 46b8bf9 Compare November 20, 2015 22:21

tkelman changed the title ~~Fix MSVC build of threading code~~ WIP: Fix MSVC build of threading code Nov 21, 2015

tkelman closed this Nov 21, 2015

tkelman deleted the tk/msvc-threads branch November 21, 2015 19:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Fix MSVC build of threading code #13970

WIP: Fix MSVC build of threading code #13970

tkelman commented Nov 13, 2015

tkelman commented Nov 13, 2015

yuyichao commented Nov 13, 2015

tkelman commented Nov 13, 2015

vtjnash commented Nov 13, 2015

yuyichao commented Nov 13, 2015

tkelman commented Nov 13, 2015

vtjnash commented Nov 13, 2015

vtjnash Nov 15, 2015

vtjnash Nov 19, 2015

tkelman Nov 19, 2015

tkelman Nov 19, 2015

tkelman Nov 20, 2015

vtjnash Nov 20, 2015

tkelman Nov 21, 2015

tkelman commented Nov 21, 2015

WIP: Fix MSVC build of threading code #13970

WIP: Fix MSVC build of threading code #13970

Conversation

tkelman commented Nov 13, 2015

tkelman commented Nov 13, 2015

yuyichao commented Nov 13, 2015

tkelman commented Nov 13, 2015

vtjnash commented Nov 13, 2015

yuyichao commented Nov 13, 2015

tkelman commented Nov 13, 2015

vtjnash commented Nov 13, 2015

vtjnash Nov 15, 2015

Choose a reason for hiding this comment

vtjnash Nov 19, 2015

Choose a reason for hiding this comment

tkelman Nov 19, 2015

Choose a reason for hiding this comment

tkelman Nov 19, 2015

Choose a reason for hiding this comment

tkelman Nov 20, 2015

Choose a reason for hiding this comment

vtjnash Nov 20, 2015

Choose a reason for hiding this comment

tkelman Nov 21, 2015

Choose a reason for hiding this comment

tkelman commented Nov 21, 2015