added 'fence' instructions to the barrier

3 files changed, 24 insertions, 39 deletions

diff --git a/lib/pure/concurrency/threadpool.nim b/lib/pure/concurrency/threadpool.nim
index 8129d03ae..c4ed42c05 100644
--- a/lib/pure/concurrency/threadpool.nim
+++ b/lib/pure/concurrency/threadpool.nim
@@ -53,12 +53,15 @@ type
     interest: bool ## wether the master is interested in the "all done" event
 
 proc barrierEnter(b: ptr Barrier) {.compilerProc, inline.} =
-  ## due to the signaling between threads, it is ensured we are the only
-  ## one with access to 'entered' so we don't need 'atomicInc' here:
+  # due to the signaling between threads, it is ensured we are the only
+  # one with access to 'entered' so we don't need 'atomicInc' here:
   inc b.entered
+  # also we need no 'fence' instructions here as soon 'nimArgsPassingDone'
+  # will be called which already will perform a fence for us.
 
 proc barrierLeave(b: ptr Barrier) {.compilerProc, inline.} =
   atomicInc b.left
+  when not defined(x86): fence()
   if b.interest and b.left == b.entered: signal(b.cv)
 
 proc openBarrier(b: ptr Barrier) {.compilerProc, inline.} =
@@ -67,10 +70,12 @@ proc openBarrier(b: ptr Barrier) {.compilerProc, inline.} =
   b.interest = false
 
 proc closeBarrier(b: ptr Barrier) {.compilerProc.} =
+  fence()
   if b.left != b.entered:
     b.cv = createCondVar()
-    b.interest = true # XXX we really need to ensure no re-orderings are done
-                      # by the C compiler here
+    fence()
+    b.interest = true
+    fence()
     while b.left != b.entered: await(b.cv)
     destroyCondVar(b.cv)
 
@@ -207,9 +212,9 @@ proc `^`*[T](prom: Promise[T]): T =
     result = prom.blob
 
 proc awaitAny*(promises: openArray[RawPromise]): int =
-  # awaits any of the given promises. Returns the index of one promise for which
-  ## a value arrived. A promise only supports one call to 'awaitAny' at the
-  ## same time. That means if you await([a,b]) and await([b,c]) the second
+  ## awaits any of the given promises. Returns the index of one promise for
+  ## which a value arrived. A promise only supports one call to 'awaitAny' at
+  ## the same time. That means if you await([a,b]) and await([b,c]) the second
   ## call will only await 'c'. If there is no promise left to be able to wait
   ## on, -1 is returned.
   ## **Note**: This results in non-deterministic behaviour and so should be
@@ -294,14 +299,16 @@ proc preferSpawn*(): bool =
 proc spawn*(call: expr): expr {.magic: "Spawn".}
   ## always spawns a new task, so that the 'call' is never executed on
   ## the calling thread. 'call' has to be proc call 'p(...)' where 'p'
-  ## is gcsafe and has 'void' as the return type.
+  ## is gcsafe and has a return type that is either 'void' or compatible
+  ## with ``Promise[T]``.
 
 template spawnX*(call: expr): expr =
   ## spawns a new task if a CPU core is ready, otherwise executes the
   ## call in the calling thread. Usually it is advised to
   ## use 'spawn' in order to not block the producer for an unknown
   ## amount of time. 'call' has to be proc call 'p(...)' where 'p'
-  ## is gcsafe and has 'void' as the return type.
+  ## is gcsafe and has a return type that is either 'void' or compatible
+  ## with ``Promise[T]``.
   (if preferSpawn(): spawn call else: call)
 
 proc parallel*(body: stmt) {.magic: "Parallel".}
diff --git a/lib/system/atomics.nim b/lib/system/atomics.nim
index 6e2bd9a97..43b3f0438 100644
--- a/lib/system/atomics.nim
+++ b/lib/system/atomics.nim
@@ -10,7 +10,9 @@
 ## Atomic operations for Nimrod.
 {.push stackTrace:off.}
 
-when (defined(gcc) or defined(llvm_gcc)) and hasThreadSupport:
+const someGcc = defined(gcc) or defined(llvm_gcc) or defined(clang)
+
+when someGcc and hasThreadSupport:
   type 
     AtomMemModel* = enum
       ATOMIC_RELAXED,  ## No barriers or synchronization. 
@@ -163,33 +165,6 @@ else:
     inc(p[], val)
     result = p[]
 
-# atomic compare and swap (CAS) funcitons to implement lock-free algorithms  
-      
-#if defined(windows) and not defined(gcc) and hasThreadSupport:
-#    proc InterlockedCompareExchangePointer(mem: ptr pointer,
-#      newValue: pointer, comparand: pointer) : pointer {.nodecl, 
-#        importc: "InterlockedCompareExchangePointer", header:"windows.h".}
-
-#    proc compareAndSwap*[T](mem: ptr T, 
-#      expected: T, newValue: T): bool {.inline.}=
-#      ## Returns true if successfully set value at mem to newValue when value
-#      ## at mem == expected
-#      return InterlockedCompareExchangePointer(addr(mem), 
-#        addr(newValue), addr(expected))[] == expected
-    
-#elif not hasThreadSupport:
-#  proc compareAndSwap*[T](mem: ptr T, 
-#                          expected: T, newValue: T): bool {.inline.} =
-#      ## Returns true if successfully set value at mem to newValue when value
-#      ## at mem == expected
-#      var oldval = mem[]
-#      if oldval == expected:
-#        mem[] = newValue
-#        return true
-#      return false
-
-
-# Some convenient functions 
 proc atomicInc*(memLoc: var int, x: int = 1): int =
   when defined(gcc) and hasThreadSupport:
     result = atomic_add_fetch(memLoc.addr, x, ATOMIC_RELAXED)
@@ -207,7 +182,7 @@ proc atomicDec*(memLoc: var int, x: int = 1): int =
     dec(memLoc, x)
     result = memLoc
 
-when defined(windows) and not defined(gcc):
+when defined(windows) and not someGcc:
   proc interlockedCompareExchange(p: pointer; exchange, comparand: int32): int32
     {.importc: "InterlockedCompareExchange", header: "<windows.h>", cdecl.}
 
@@ -221,7 +196,7 @@ else:
   # XXX is this valid for 'int'?
 
 
-when (defined(x86) or defined(amd64)) and defined(gcc):
+when (defined(x86) or defined(amd64)) and (defined(gcc) or defined(llvm_gcc)):
   proc cpuRelax {.inline.} =
     {.emit: """asm volatile("pause" ::: "memory");""".}
 elif (defined(x86) or defined(amd64)) and defined(vcc):
diff --git a/todo.txt b/todo.txt
index 8a351e8a7..7d4eac1ad 100644
--- a/todo.txt
+++ b/todo.txt
@@ -8,8 +8,11 @@ Concurrency
 - implement 'deepCopy' builtin
 - implement 'foo[1..4] = spawn(f[4..7])'
 - the disjoint checker needs to deal with 'a = spawn f(); g = spawn f()'
+- support for exception propagation
 - Minor: The copying of the 'ref Promise' into the thead local storage only
   happens to work due to the write barrier's implementation
+- 'gcsafe' inferrence needs to be fixed
+- implement lock levels --> first without the more complex race avoidance
 
 
 Misc