Threading without using patched LLVM #14083

yuyichao · 2015-11-21T16:03:12Z

This tries to implement what @vtjnash suggested for making threading working without LLVM patch. This should also make it possible to support threading on windows since (I was told that) the LLVM tls patch doesn't support emitting TLS on windows.

A few notes ~~and TODOs~~ (all done):

The first few commits in this PR are not directly related to TLS, they are either slightly related
clean-up's or fix of the bug discovered when testing this PR.
(merged as Preserve function attributes in prepare_call (plus clean up) #14084)
This is currently GCC/Clang only due to the __attribute__((const)) but it is only for performance
and not for correctness so can easily be disabled for other compilers (MSVC) or replaced with
equivalent attributes. (fixed, MSVC doesn't/won't support it, use __declspec(noalias) on MSVC)
This placed all TLS variables in a big struct. Mainly because it's easier to do... There's no #ifdef's
in the struct in order to keep it clean and to make the threading and non-threading versions ABI
compatible (to certain degree anyway...). This also effectively export all the TLS variables but we
could easily fix this using a nested struct (and only exposing the pointer to the inner, exported
struct) if it is a concern. (FWIW, this is C and there's way more things you can do to break stuff
other than accessing a non-exported TLS variable....)
Compilation passes, REPL works and all tests passes for both threading and non-threading on
LLVM37. (The compilation of inference0.ji with threading on had one SegFault that I couldn't
reproduce). (reported as Race condition in threads initialization. #14091)
Codegen is always accessing the TLS variables through a (const) function call. I get a freeze
before enterning the REPL if I let it use the global variable directly for non-threading version.
(probably because I didn't declare the struct correctly for LLVM). (fixed). OTOH, with the
function attributes set (copied from Clang), llvm is able to emit only on call to jl_get_ptls_states()
per function.
~~TLS model is still global-dynamic.~~ Using a dynamic model by default. Override to a static model in repl. The override has to be done at initialization time and can be done for embedding as well if needed.

c.c @tkelman @Keno

edit: closes #13975

tkelman · 2015-11-21T22:35:46Z

Lovely, thank you for this. This will close #13975. I wonder whether this would allow us to turn threading on by default before we move to newer LLVM? First time I tried that on win64 it segfaulted in bootstrap, though will try another few times to see if it's the same problem as #14091.

yuyichao · 2015-11-21T22:54:23Z

This will close #13975

I suspect #13975 is the same with #14091 (since it segfault before the LLVM error)

yuyichao · 2015-11-22T05:31:38Z

Squashed and all the TODO's in this PR are done now! Remove WIP from the title and this PR is ready for review.

This essentially implements a runtime weak symbol. It would be nice if the dynamic linker can do this for us (I tried __attribute__((weak)) but couldn't get it working).

Getting the address of the TLS variable is not as fast as (~30-60% slower) the optimal case (a single function returning a static TLS address) but is also much faster (2x-3x) than using the global dynamic model. Note that this is also only relavant for code in the sys.so, libjulia.so and julia-ui (basically any binary code we save to/load from disk), JIT code use the actually getter function address directly. The impact on the overall performance will also be much smaller since we only emit TLS variables in expensive functions (gcframe, catch exceptions).

The advantage of this approach is that it should be able to support all platforms (non-linux have a lot of issues with unresolved symbols in a shared library) and doesn't require another shim for embedding.

vtjnash · 2015-11-22T17:05:36Z

src/codegen.cpp

@@ -5640,6 +5625,25 @@ static void init_julia_llvm_env(Module *m)
                                     "jl_float_temp"));
 #endif

+#ifndef JULIA_ENABLE_THREADING
+    jltls_states_var =
+        new GlobalVariable(*m, ArrayType::get(T_int8, sizeof(jl_tls_states_t)),


I think this can be declared as an ArrayType of T_pint8 so that alignment is correct and to avoid the BitCastOp on most usages

I actually had that at first but was a little bit paranoid about the size I should supply since jl_tls_states_t has a int16_t and is not exactly multiple of pointer size.

true, although although sizeof must be a multiple of __alignof in C, so the over-allocation is guaranteed

Agree, I'll rearange the members a little and declare it as a pointer array to LLVM.

Done. You are right that sizeof(jl_tls_states_t) % sizeof(void*) == 0. The bitcast was actually not necessary anyway since it's always casted to T_ppjlvalue by emit_nthptr_addr anyway....

vtjnash · 2015-11-22T17:48:13Z

lgtm. thanks, 🎅 🐧

fixes 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 also define sleep for MSVC Kernel32 Sleep takes milliseconds, ref https://msdn.microsoft.com/en-us/library/windows/desktop/ms686298(v=vs.85).aspx fixes d:/code/msys64/home/Tony/julia/src/threading.c(212) : error C3861: 'sleep': iden tifier not found d:/code/msys64/home/Tony/julia/src/threading.c(290) : error C3861: 'sleep': iden tifier not found

…e always `extern "C"`'d so codegen_mutex and codegen_lock_count do not get c++ mangled

yuyichao · 2015-11-23T16:29:33Z

So I've just run some micro-benchmarks to compare the performance of creating a useless gc frame. The functions I used to benchmark this is (they are the gcframe and no gcframe respectively in the table below).

@noinline function tls_test(a)
    1 > 2 && "a$a"
    a
end

@noinline function non_tls_test(a)
    a
end

The first function generates a useless gcframe for the branch eliminated by llvm.

I've also precompiled these in the sysimg and verified in GDB that for this PR with threading, the sysimg version has one more level of indirection through jl_get_ptls_states while the JIT version calls jl_get_ptls_states_static directly.

Numbers below (time in ns)

	master	this pr	master threading	this pr threading
gcframe (sysimg)	1.74	1.74	5.22	3.07
no gcframe (sysimg)	1.09	1.09	1.09	1.09
gcframe	1.53	1.72	5.22	2.93
no gcframe	1.09	1.09	1.09	1.09

The only possible regression is the non-threading GC frame is slower by 1-2 cycles (0.217ns). The extra level of indirection for tls access in sysimg isn't as bad as what I thought.

Overall, I'm pretty happy with the current performance. Maybe we can do better for the tls access (trick the linker or emitting static TLS access if we can recognize that (with llvm support)?) but not for this PR ;-p

Will merge tomorrow if there's no other issues.

tkelman · 2015-11-23T20:28:15Z

Let's merge now?

Threading without using patched LLVM

yuyichao · 2015-11-23T20:50:31Z

Sure.....

ViralBShah · 2015-11-24T16:03:34Z

Does this mean we can turn on travis?

yuyichao · 2015-11-24T16:05:57Z

The build should work, but I'm pretty sure there's still bugs (not sure if we can hit those bugs without actually running many threads though). Maybe at least have a buildbot for it? @tkelman @staticfloat

tkelman · 2015-11-24T16:31:01Z

It is dev master. I say let's turn threads on by default, everywhere except MSVC.

yuyichao · 2015-11-24T16:41:19Z

I'd prefer to leave it off until we have a stop-the-world GC (rather than a wait-for-the-world GC as @vtjnash phrased it....)

ViralBShah · 2015-11-24T18:43:20Z

I thought there was a buildbot for threading.

staticfloat · 2015-11-24T18:44:30Z

@yuyichao What build configuration would you want? Just tacking JULIA_ENABLE_THREADING onto the makevars?

yuyichao · 2015-11-24T18:59:16Z

I thought there was a buildbot for threading.

I thought there wasn't since I've heard there's issues getting newer LLVM to build.

What build configuration would you want

Normal build and test with JULIA_THREADS=1 make var.

staticfloat · 2015-11-24T19:03:42Z

What is the minimum version of LLVM we can build against? We have Cxx nightlies that actually finish building every now and then, but if there is a specific version we can build against that'd be great.

tkelman · 2015-11-24T19:06:02Z

Cxx nightlies are broken on gcc 5 because they are based off a broken point in upstream's history, see JuliaInterop/Cxx.jl#73 (comment).

Threading with LLVM 3.3 should now work, which was the purpose of this PR.

ViralBShah · 2015-11-24T19:06:46Z

Perhaps I was thinking of the cxx buildbot. Sorry for the noise.

staticfloat · 2015-11-24T19:12:56Z

Alright, I've got a nightly builder that will attempt this stuff, and have a test build forcing its way through right now.

yuyichao · 2015-11-24T19:17:09Z

Seems like it was broken by #14103 (comment)

yuyichao mentioned this pull request Nov 21, 2015

Preserve function attributes in prepare_call (plus clean up) #14084

Merged

yuyichao force-pushed the yyc/tls-no-patch branch 6 times, most recently from 67a6369 to 1bd69dd Compare November 21, 2015 18:49

tkelman mentioned this pull request Nov 21, 2015

WIP: Fix MSVC build of threading code #13970

Closed

yuyichao force-pushed the yyc/tls-no-patch branch from 1bd69dd to 393bddc Compare November 21, 2015 21:23

yuyichao mentioned this pull request Nov 21, 2015

Race condition in threads initialization. #14091

Closed

tkelman added the multithreading Base.Threads and related functionality label Nov 21, 2015

yuyichao force-pushed the yyc/tls-no-patch branch 2 times, most recently from 2ecc7e5 to b2339b8 Compare November 22, 2015 05:27

yuyichao changed the title ~~WIP: threading without using patched LLVM~~ Threading without using patched LLVM Nov 22, 2015

yuyichao force-pushed the yyc/tls-no-patch branch from 70c2643 to c8ae160 Compare November 22, 2015 13:11

yuyichao mentioned this pull request Nov 22, 2015

Try to synchronize threads initializations #14098

Closed

vtjnash reviewed Nov 22, 2015
View reviewed changes

yuyichao and others added 6 commits November 22, 2015 20:26

Threading without LLVM patch

11e526f

Move JL_DEFINE_MUTEX_EXT(codegen) to julia_internal.h so that they ar…

103cd89

…e always `extern "C"`'d so codegen_mutex and codegen_lock_count do not get c++ mangled

Allow the tls state to be overrided (to a faster one)

5ceb218

Use the tls states cb function directly for non-imaging code

06a255b

Make sure the tls callback is always initialized before threads starts.

2986358

yuyichao force-pushed the yyc/tls-no-patch branch from c8ae160 to 2986358 Compare November 23, 2015 01:32

yuyichao mentioned this pull request Nov 23, 2015

use sigwait for thread-safe and multi-thread-aware signal handling #12904

Merged

yuyichao added a commit that referenced this pull request Nov 23, 2015

Merge pull request #14083 from JuliaLang/yyc/tls-no-patch

e6aba04

Threading without using patched LLVM

yuyichao merged commit e6aba04 into master Nov 23, 2015

yuyichao deleted the yyc/tls-no-patch branch November 23, 2015 20:40

yuyichao mentioned this pull request Nov 23, 2015

Windows build with JULIA_THREADS = 1 segfaults #13975

Closed

yuyichao mentioned this pull request Nov 27, 2015

Optimize TLS access in sysimg. #14168

Merged

yuyichao mentioned this pull request Feb 15, 2016

Remove unnecessary options for accessing TLS from JIT code #15081

Merged

yuyichao mentioned this pull request Jun 9, 2016

statepoints instead of the shadow stack #16847

Closed

3 tasks

yuyichao mentioned this pull request Jun 30, 2016

(Yet another) Optimization for TLS access #17220

Merged

yuyichao mentioned this pull request Sep 26, 2020

Introduce libjuliarepl to break dependence on runtime libraries #36588

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Threading without using patched LLVM #14083

Threading without using patched LLVM #14083

yuyichao commented Nov 21, 2015

tkelman commented Nov 21, 2015

yuyichao commented Nov 21, 2015

yuyichao commented Nov 22, 2015

vtjnash Nov 22, 2015

yuyichao Nov 22, 2015

vtjnash Nov 22, 2015

yuyichao Nov 22, 2015

yuyichao Nov 23, 2015

vtjnash commented Nov 22, 2015

yuyichao commented Nov 23, 2015

tkelman commented Nov 23, 2015

yuyichao commented Nov 23, 2015

ViralBShah commented Nov 24, 2015

yuyichao commented Nov 24, 2015

tkelman commented Nov 24, 2015

yuyichao commented Nov 24, 2015

ViralBShah commented Nov 24, 2015

staticfloat commented Nov 24, 2015

yuyichao commented Nov 24, 2015

staticfloat commented Nov 24, 2015

tkelman commented Nov 24, 2015

ViralBShah commented Nov 24, 2015

staticfloat commented Nov 24, 2015

yuyichao commented Nov 24, 2015

Threading without using patched LLVM #14083

Threading without using patched LLVM #14083

Conversation

yuyichao commented Nov 21, 2015

tkelman commented Nov 21, 2015

yuyichao commented Nov 21, 2015

yuyichao commented Nov 22, 2015

vtjnash Nov 22, 2015

Choose a reason for hiding this comment

yuyichao Nov 22, 2015

Choose a reason for hiding this comment

vtjnash Nov 22, 2015

Choose a reason for hiding this comment

yuyichao Nov 22, 2015

Choose a reason for hiding this comment

yuyichao Nov 23, 2015

Choose a reason for hiding this comment

vtjnash commented Nov 22, 2015

yuyichao commented Nov 23, 2015

tkelman commented Nov 23, 2015

yuyichao commented Nov 23, 2015

ViralBShah commented Nov 24, 2015

yuyichao commented Nov 24, 2015

tkelman commented Nov 24, 2015

yuyichao commented Nov 24, 2015

ViralBShah commented Nov 24, 2015

staticfloat commented Nov 24, 2015

yuyichao commented Nov 24, 2015

staticfloat commented Nov 24, 2015

tkelman commented Nov 24, 2015

ViralBShah commented Nov 24, 2015

staticfloat commented Nov 24, 2015

yuyichao commented Nov 24, 2015