diff options
author | Timothee Cour <timothee.cour2@gmail.com> | 2021-01-27 13:35:43 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-01-27 22:35:43 +0100 |
commit | e1129749207d25597e81fe06cd7e3099b5ecb61e (patch) | |
tree | a521faa436260c9abf30cd9bf4530f8020ad1a9f | |
parent | 3c8fddbc7698f99cd595ace9ffca0c587622088f (diff) | |
download | Nim-e1129749207d25597e81fe06cd7e3099b5ecb61e.tar.gz |
fix #16752: threadvar now works with importcpp types; osx now uses native TLS (`--tlsEmulation:off`), which can be orders of magnitude faster (#16750)
* osx now uses native TLS, which can be orders of magnitude faster * add {.cppNonPod.} * improve test * changelog, docs, disable part of windows test
-rw-r--r-- | changelog.md | 3 | ||||
-rw-r--r-- | compiler/ast.nim | 4 | ||||
-rw-r--r-- | compiler/ccgthreadvars.nim | 10 | ||||
-rw-r--r-- | compiler/options.nim | 11 | ||||
-rw-r--r-- | compiler/pragmas.nim | 4 | ||||
-rw-r--r-- | compiler/wordrecg.nim | 3 | ||||
-rw-r--r-- | config/nim.cfg | 8 | ||||
-rw-r--r-- | doc/manual.rst | 13 | ||||
-rw-r--r-- | lib/nimbase.h | 4 | ||||
-rw-r--r-- | tests/benchmarks/readme.md | 5 | ||||
-rw-r--r-- | tests/benchmarks/ttls.nim | 30 | ||||
-rw-r--r-- | tests/misc/mtlsemulation.h | 37 | ||||
-rw-r--r-- | tests/misc/ttlsemulation.nim | 75 |
13 files changed, 194 insertions, 13 deletions
diff --git a/changelog.md b/changelog.md index af979fc93..8221a157e 100644 --- a/changelog.md +++ b/changelog.md @@ -132,10 +132,13 @@ with other backends. see #9125. Use `-d:nimLegacyJsRound` for previous behavior. - Added `nim --eval:cmd` to evaluate a command directly, see `nim --help`. - VM now supports `addr(mystring[ind])` (index + index assignment) + - Type mismatch errors now show more context, use `-d:nimLegacyTypeMismatch` for previous behavior. - Added `--hintAsError` with similar semantics as `--warningAsError`. +- TLS: OSX now uses native TLS (`--tlsEmulation:off`), TLS now works with importcpp non-POD types, + such types must use `.cppNonPod` and `--tlsEmulation:off`should be used. ## Tool changes diff --git a/compiler/ast.nim b/compiler/ast.nim index c9eccadac..31d47a6fd 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -265,6 +265,10 @@ type sfShadowed, # a symbol that was shadowed in some inner scope sfThread, # proc will run as a thread # variable is a thread variable + sfCppNonPod, # tells compiler to treat such types as non-pod's, so that + # `thread_local` is used instead of `__thread` for + # {.threadvar.} + `--threads`. Only makes sense for importcpp types. + # This has a performance impact so isn't set by default. sfCompileTime, # proc can be evaluated at compile time sfConstructor, # proc is a C++ constructor sfDispatcher, # copied method symbol is the dispatcher diff --git a/compiler/ccgthreadvars.nim b/compiler/ccgthreadvars.nim index eba46f829..fbe8bce9e 100644 --- a/compiler/ccgthreadvars.nim +++ b/compiler/ccgthreadvars.nim @@ -7,8 +7,8 @@ # distribution, for details about the copyright. # -## Thread var support for crappy architectures that lack native support for -## thread local storage. (**Thank you Mac OS X!**) +## Thread var support for architectures that lack native support for +## thread local storage. # included from cgen.nim @@ -35,7 +35,11 @@ proc declareThreadVar(m: BModule, s: PSym, isExtern: bool) = if isExtern: m.s[cfsVars].add("extern ") elif lfExportLib in s.loc.flags: m.s[cfsVars].add("N_LIB_EXPORT_VAR ") else: m.s[cfsVars].add("N_LIB_PRIVATE ") - if optThreads in m.config.globalOptions: m.s[cfsVars].add("NIM_THREADVAR ") + if optThreads in m.config.globalOptions: + let sym = s.typ.sym + if sym != nil and sfCppNonPod in sym.flags: + m.s[cfsVars].add("NIM_THREAD_LOCAL ") + else: m.s[cfsVars].add("NIM_THREADVAR ") m.s[cfsVars].add(getTypeDesc(m, s.loc.t)) m.s[cfsVars].addf(" $1;$n", [s.loc.r]) diff --git a/compiler/options.nim b/compiler/options.nim index 425cf9c7f..3cc2899d7 100644 --- a/compiler/options.nim +++ b/compiler/options.nim @@ -380,11 +380,12 @@ proc hasWarn*(conf: ConfigRef, note: TNoteKind): bool = proc hcrOn*(conf: ConfigRef): bool = return optHotCodeReloading in conf.globalOptions -template depConfigFields*(fn) {.dirty.} = - fn(target) - fn(options) - fn(globalOptions) - fn(selectedGC) +when false: + template depConfigFields*(fn) {.dirty.} = # deadcode + fn(target) + fn(options) + fn(globalOptions) + fn(selectedGC) const oldExperimentalFeatures* = {implicitDeref, dotOperators, callOperator, parallel} diff --git a/compiler/pragmas.nim b/compiler/pragmas.nim index c8aee3327..50b8c338f 100644 --- a/compiler/pragmas.nim +++ b/compiler/pragmas.nim @@ -65,7 +65,7 @@ const wPure, wHeader, wCompilerProc, wCore, wFinal, wSize, wShallow, wIncompleteStruct, wCompleteStruct, wByCopy, wByRef, wInheritable, wGensym, wInject, wRequiresInit, wUnchecked, wUnion, wPacked, - wBorrow, wGcSafe, wPartial, wExplain, wPackage} + wCppNonPod, wBorrow, wGcSafe, wPartial, wExplain, wPackage} fieldPragmas* = declPragmas + { wGuard, wBitsize, wCursor, wRequiresInit, wNoalias} - {wExportNims, wNodecl} # why exclude these? varPragmas* = declPragmas + {wVolatile, wRegister, wThreadVar, @@ -843,6 +843,8 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int, else: invalidPragma(c, it) of wImportCpp: processImportCpp(c, sym, getOptionalStr(c, it, "$1"), it.info) + of wCppNonPod: + incl(sym.flags, sfCppNonPod) of wImportJs: if c.config.backend != backendJs: localError(c.config, it.info, "`importjs` pragma requires the JavaScript target") diff --git a/compiler/wordrecg.nim b/compiler/wordrecg.nim index ad529f437..6a68e2c70 100644 --- a/compiler/wordrecg.nim +++ b/compiler/wordrecg.nim @@ -38,7 +38,8 @@ type wCursor = "cursor", wNoalias = "noalias", wImmediate = "immediate", wConstructor = "constructor", wDestructor = "destructor", - wDelegator = "delegator", wOverride = "override", wImportCpp = "importcpp", + wDelegator = "delegator", wOverride = "override", wImportCpp = "importcpp", + wCppNonPod = "cppNonPod", wImportObjC = "importobjc", wImportCompilerProc = "importcompilerproc", wImportc = "importc", wImportJs = "importjs", wExportc = "exportc", wExportCpp = "exportcpp", wExportNims = "exportnims", diff --git a/config/nim.cfg b/config/nim.cfg index 83caf8a2b..3e7362a36 100644 --- a/config/nim.cfg +++ b/config/nim.cfg @@ -114,8 +114,6 @@ path="$lib/pure" @if bsd: # BSD got posix_spawn only recently, so we deactivate it for osproc: define:useFork - # at least NetBSD has problems with thread local storage: - tlsEmulation:on @elif haiku: gcc.options.linker = "-Wl,--as-needed -lnetwork" gcc.cpp.options.linker = "-Wl,--as-needed -lnetwork" @@ -167,9 +165,13 @@ path="$lib/pure" gcc.maxerrorsimpl = "-fmax-errors=3" +@if bsd: + # at least NetBSD has problems with thread local storage: + tlsEmulation:on +@end + @if macosx or freebsd or openbsd: cc = clang - tlsEmulation:on gcc.options.always %= "-w ${gcc.maxerrorsimpl}" gcc.cpp.options.always %= "-w ${gcc.maxerrorsimpl} -fpermissive" @elif windows: diff --git a/doc/manual.rst b/doc/manual.rst index 929aaec6a..2ba05ed4e 100644 --- a/doc/manual.rst +++ b/doc/manual.rst @@ -7214,6 +7214,19 @@ will generate this code: __interrupt void myinterrupt() +`cppNonPod` pragma +------------------ + +The `.cppNonPod` pragma should be used for non-POD `importcpp` types so that they +work properly (in particular regarding constructor and destructor) for +`.threadvar` variables. This requires `--tlsEmulation:off`. + +.. code-block:: nim + type Foo {.cppNonPod, importcpp, header: "funs.h".} = object + x: cint + proc main()= + var a {.threadvar.}: Foo + InjectStmt pragma ----------------- diff --git a/lib/nimbase.h b/lib/nimbase.h index daae47d65..cbd35605b 100644 --- a/lib/nimbase.h +++ b/lib/nimbase.h @@ -143,6 +143,10 @@ __AVR__ # error "Cannot define NIM_THREADVAR" #endif +#if defined(__cplusplus) + #define NIM_THREAD_LOCAL thread_local +#endif + /* --------------- how int64 constants should be declared: ----------- */ #if defined(__GNUC__) || defined(__LCC__) || \ defined(__POCC__) || defined(__DMC__) || defined(_MSC_VER) diff --git a/tests/benchmarks/readme.md b/tests/benchmarks/readme.md new file mode 100644 index 000000000..1e744fc40 --- /dev/null +++ b/tests/benchmarks/readme.md @@ -0,0 +1,5 @@ +# Collection of benchmarks + +In future work, benchmarks can be added to CI, but for now we provide benchmarks that can be run locally. + +See RFC: https://github.com/timotheecour/Nim/issues/425 diff --git a/tests/benchmarks/ttls.nim b/tests/benchmarks/ttls.nim new file mode 100644 index 000000000..f5314850f --- /dev/null +++ b/tests/benchmarks/ttls.nim @@ -0,0 +1,30 @@ +discard """ + action: compile +""" + +#[ +## on osx +nim r -d:danger --threads --tlsEmulation:off tests/benchmarks/ttls.nim +9.999999999992654e-07 + +ditto with `--tlsEmulation:on`: +0.216999 +]# + +import times + +proc main2(): int = + var g0 {.threadvar.}: int + g0.inc + result = g0 + +proc main = + let n = 100_000_000 + var c = 0 + let t = cpuTime() + for i in 0..<n: + c += main2() + let t2 = cpuTime() - t + doAssert c != 0 + echo t2 +main() diff --git a/tests/misc/mtlsemulation.h b/tests/misc/mtlsemulation.h new file mode 100644 index 000000000..992977acd --- /dev/null +++ b/tests/misc/mtlsemulation.h @@ -0,0 +1,37 @@ +#include <stdio.h> + +struct Foo1 { + /* + uncommenting would give: + error: initializer for thread-local variable must be a constant expression + N_LIB_PRIVATE NIM_THREADVAR Foo1 g1__9brEZhPEldbVrNpdRGmWESA; + */ + // Foo1() noexcept { } + + /* + uncommenting would give: + error: type of thread-local variable has non-trivial destruction + */ + // ~Foo1() { } + int x; +}; + +struct Foo2 { + Foo2() noexcept { } + ~Foo2() { } + int x; +}; + +static int ctorCalls = 0; +static int dtorCalls = 0; + +struct Foo3 { + Foo3() noexcept { + ctorCalls = ctorCalls + 1; + x = 10; + } + ~Foo3() { + dtorCalls = dtorCalls + 1; + } + int x; +}; diff --git a/tests/misc/ttlsemulation.nim b/tests/misc/ttlsemulation.nim new file mode 100644 index 000000000..47c5934e6 --- /dev/null +++ b/tests/misc/ttlsemulation.nim @@ -0,0 +1,75 @@ +discard """ + matrix: "-d:nimTtlsemulationCase1 --threads --tlsEmulation:on; -d:nimTtlsemulationCase2 --threads --tlsEmulation:off; -d:nimTtlsemulationCase3 --threads" + targets: "c cpp" +""" + +#[ +tests for: `.cppNonPod`, `--tlsEmulation` +]# + +import std/sugar + +block: + # makes sure the logic in config/nim.cfg or testament doesn't interfere with `--tlsEmulation` so we test the right thing. + when defined(nimTtlsemulationCase1): + doAssert compileOption("tlsEmulation") + elif defined(nimTtlsemulationCase2): + doAssert not compileOption("tlsEmulation") + elif defined(nimTtlsemulationCase3): + when defined(osx): + doAssert not compileOption("tlsEmulation") + else: + doAssert false + +block: + proc main1(): int = + var g0 {.threadvar.}: int + g0.inc + g0 + let s = collect: + for i in 0..<3: main1() + doAssert s == @[1,2,3] + +when defined(cpp): # bug #16752 + when defined(windows) and defined(nimTtlsemulationCase2): + discard # xxx this failed with exitCode 1 + else: + type Foo1 {.importcpp: "Foo1", header: "mtlsemulation.h".} = object + x: cint + type Foo2 {.cppNonPod, importcpp: "Foo2", header: "mtlsemulation.h".} = object + x: cint + + var ctorCalls {.importcpp.}: cint + var dtorCalls {.importcpp.}: cint + type Foo3 {.cppNonPod, importcpp: "Foo3", header: "mtlsemulation.h".} = object + x: cint + + proc sub(i: int) = + var g1 {.threadvar.}: Foo1 + var g2 {.threadvar.}: Foo2 + var g3 {.threadvar.}: Foo3 + discard g1 + discard g2 + + # echo (g3.x, ctorCalls, dtorCalls) + when compileOption("tlsEmulation"): + # xxx bug + discard + else: + doAssert g3.x.int == 10 + i + doAssert ctorCalls == 2 + doAssert dtorCalls == 1 + g3.x.inc + + proc main() = + doAssert ctorCalls == 0 + doAssert dtorCalls == 0 + block: + var f3: Foo3 + doAssert f3.x == 10 + doAssert ctorCalls == 1 + doAssert dtorCalls == 1 + + for i in 0..<3: + sub(i) + main() |