Skip to content
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

Atomic symbol resolution: unable to use std/atomics #12695

Open
mratsim opened this issue Nov 21, 2019 · 10 comments
Open

Atomic symbol resolution: unable to use std/atomics #12695

mratsim opened this issue Nov 21, 2019 · 10 comments

Comments

@mratsim
Copy link
Collaborator

mratsim commented Nov 21, 2019

I manage to reach an atomic dead-end with the long series of symbol resolution issues #8677

The code snippet at the bottom triggers on devel and 1.0.2 the following error messages:

.../debug/file1.nim(18, 15) template/generic instantiation of `trySend` from here
.../debug/file1.nim(12, 12) template/generic instantiation of `store` from here
.../.choosenim/toolchains/nim-#devel/lib/pure/concurrency/atomics.nim(302, 42) Error: undeclared field: 'value'

The relevant atomics are:

Atomic*[T] = object
when T is Trivial:
value: T.atomicType
else:
nonAtomicValue: T
guard: AtomicFlag
#proc init*[T](location: var Atomic[T]; value: T): T {.importcpp: "atomic_init(@)".}
proc atomic_load_explicit[T, A](location: ptr A; order: MemoryOrder): T {.importc.}
proc atomic_store_explicit[T, A](location: ptr A; desired: T; order: MemoryOrder = moSequentiallyConsistent) {.importc.}
proc atomic_exchange_explicit[T, A](location: ptr A; desired: T; order: MemoryOrder = moSequentiallyConsistent): T {.importc.}
proc atomic_compare_exchange_strong_explicit[T, A](location: ptr A; expected: ptr T; desired: T; success, failure: MemoryOrder): bool {.importc.}
proc atomic_compare_exchange_weak_explicit[T, A](location: ptr A; expected: ptr T; desired: T; success, failure: MemoryOrder): bool {.importc.}
# Numerical operations
proc atomic_fetch_add_explicit[T, A](location: ptr A; value: T; order: MemoryOrder = moSequentiallyConsistent): T {.importc.}
proc atomic_fetch_sub_explicit[T, A](location: ptr A; value: T; order: MemoryOrder = moSequentiallyConsistent): T {.importc.}
proc atomic_fetch_and_explicit[T, A](location: ptr A; value: T; order: MemoryOrder = moSequentiallyConsistent): T {.importc.}
proc atomic_fetch_or_explicit[T, A](location: ptr A; value: T; order: MemoryOrder = moSequentiallyConsistent): T {.importc.}
proc atomic_fetch_xor_explicit[T, A](location: ptr A; value: T; order: MemoryOrder = moSequentiallyConsistent): T {.importc.}
# Flag operations
# var ATOMIC_FLAG_INIT {.importc, nodecl.}: AtomicFlag
# proc init*(location: var AtomicFlag) {.inline.} = location = ATOMIC_FLAG_INIT
proc testAndSet*(location: var AtomicFlag; order: MemoryOrder = moSequentiallyConsistent): bool {.importc: "atomic_flag_test_and_set_explicit".}
proc clear*(location: var AtomicFlag; order: MemoryOrder = moSequentiallyConsistent) {.importc: "atomic_flag_clear_explicit".}
proc fence*(order: MemoryOrder) {.importc: "atomic_thread_fence".}
proc signalFence*(order: MemoryOrder) {.importc: "atomic_signal_fence".}
{.pop.}
proc load*[T: Trivial](location: var Atomic[T]; order: MemoryOrder = moSequentiallyConsistent): T {.inline.} =
cast[T](atomic_load_explicit[nonAtomicType(T), type(location.value)](addr(location.value), order))
proc store*[T: Trivial](location: var Atomic[T]; desired: T; order: MemoryOrder = moSequentiallyConsistent) {.inline.} =
atomic_store_explicit(addr(location.value), cast[nonAtomicType(T)](desired), order)

Value is defined at line 269, exporting or mixin or bind don't work

import std/atomics

type
  MyType* = ptr object
    next*: Atomic[MyType]

  ChannelFoo*[T] = object
    front: T
    back: Atomic[T]

proc trySend*[T](chan: var ChannelFoo[T], src: sink T): bool =
  chan.back.store(nil, moRelaxed)


var x = createShared(typeof(default(MyType)[]))
var c = createShared(ChannelFoo[MyType])

let done = c[].trySend(x)
@mratsim
Copy link
Collaborator Author

mratsim commented Nov 21, 2019

Discussing with @Clyybber I realised what was different from my 4 other channels implementation at https://github.com/mratsim/weave/tree/b8ac8d64ca07eb47a926f3f70b96e94fe930cf08/weave/channels. It's the only one using non-instantiated generics. The others uses explicit Atomic[bool] and Atomic[int].

I can workaround this limitation by using Atomic[pointer] instead of Atomic[T] and casting, it's a bit annoying though especially for var parameters as var ptr T are not auto-converted to var pointer but to simple pointer (which I am not against mind you)

@Clyybber
Copy link
Contributor

Clyybber commented Nov 21, 2019

This also works:

import std/atomics

type
  Trivial = SomeNumber | bool | ptr | pointer
  MyType* = ptr object
    next*: Atomic[MyType]

  ChannelFoo*[T: Trivial] = object
    front: T
    back: Atomic[T]

proc trySend*[T: Trivial](chan: var ChannelFoo[T], src: sink T): bool =
  chan.back.store(nil, moRelaxed)


var x = createShared(typeof(default(MyType)[]))
var c = createShared(ChannelFoo[MyType])

let done = c[].trySend(x)

EDIT: Actually crashes :(

@mratsim
Copy link
Collaborator Author

mratsim commented Nov 21, 2019

Final ugly workaround as I need an Enqueueable concept.

import std/atomics

type
  MyType* = ptr object
    next*: Atomic[MyType]

  Trivial = ptr | pointer

  Enqueueable = concept x, type T
    x is Trivial
    x.next is Atomic[T]

  ChannelFoo*[T: Trivial and Enqueueable] = object
    front: T
    back: Atomic[T]

proc trySend*[T: Trivial and Enqueueable](chan: var ChannelFoo[T], src: sink T): bool =
  chan.back.store(nil, moRelaxed)

var x = createShared(typeof(default(MyType)[]))
var c = createShared(ChannelFoo[MyType])

let done = c[].trySend(x)

Just restraining via concept doesn't workaround the issue. Also "TrySend" must be constrained as well

@mratsim
Copy link
Collaborator Author

mratsim commented Nov 21, 2019

Actually I was tricked, that makes the compiler crash without message :/

Traceback (most recent call last)
/home/beta/Programming/Nim/Nim/compiler/nim.nim(106) nim
/home/beta/Programming/Nim/Nim/compiler/nim.nim(83) handleCmdLine
/home/beta/Programming/Nim/Nim/compiler/cmdlinehelper.nim(98) loadConfigsAndRunMainCommand
/home/beta/Programming/Nim/Nim/compiler/main.nim(188) mainCommand
/home/beta/Programming/Nim/Nim/compiler/main.nim(92) commandCompileToC
/home/beta/Programming/Nim/Nim/compiler/modules.nim(144) compileProject
/home/beta/Programming/Nim/Nim/compiler/modules.nim(85) compileModule
/home/beta/Programming/Nim/Nim/compiler/passes.nim(213) processModule
/home/beta/Programming/Nim/Nim/compiler/passes.nim(86) processTopLevelStmt
/home/beta/Programming/Nim/Nim/compiler/sem.nim(601) myProcess
/home/beta/Programming/Nim/Nim/compiler/sem.nim(569) semStmtAndGenerateGenerics
/home/beta/Programming/Nim/Nim/compiler/semstmts.nim(2221) semStmt
/home/beta/Programming/Nim/Nim/compiler/semexprs.nim(987) semExprNoType
/home/beta/Programming/Nim/Nim/compiler/semexprs.nim(2760) semExpr
/home/beta/Programming/Nim/Nim/compiler/semstmts.nim(1996) semProc
/home/beta/Programming/Nim/Nim/compiler/semstmts.nim(1810) semProcAux
/home/beta/Programming/Nim/Nim/compiler/semstmts.nim(1366) semParamList
/home/beta/Programming/Nim/Nim/compiler/semtypes.nim(1149) semProcTypeNode
/home/beta/Programming/Nim/Nim/compiler/semtypes.nim(1108) semParamType
/home/beta/Programming/Nim/Nim/compiler/semtypes.nim(1781) semTypeNode
/home/beta/Programming/Nim/Nim/compiler/semtypes.nim(188) semVarType
/home/beta/Programming/Nim/Nim/compiler/semtypes.nim(1704) semTypeNode
/home/beta/Programming/Nim/Nim/compiler/semtypes.nim(1371) semGeneric
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(2522) matches
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(2460) matchesAux
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(2170) paramTypesMatch
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(2014) paramTypesMatchAux
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(1680) typeRel
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(1097) typeRel
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(1572) typeRel
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(1075) typeRel
(1874 calls omitted) ...
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(1075) typeRel
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(1075) typeRel
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(1075) typeRel
[...]
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(1075) typeRel
/home/beta/Programming/Nim/Nim/compiler/sigmatch.nim(1073) typeRel
/home/beta/Programming/Nim/Nim/compiler/astalgo.nim(899) idTableGet
/home/beta/Programming/Nim/Nim/compiler/astalgo.nim idTableRawGet
Error: call depth limit reached in a debug build (2000 function calls). You can change it with -d:nimCallDepthLimit=<int> but really try to avoid deep recursions instead.
FAILURE

@mratsim
Copy link
Collaborator Author

mratsim commented Nov 21, 2019

Actually even your solution crashes

mratsim added a commit to mratsim/weave that referenced this issue Nov 22, 2019
mratsim added a commit to mratsim/weave that referenced this issue Nov 22, 2019
mratsim added a commit to mratsim/weave that referenced this issue Nov 24, 2019
* Stashing: Sadly, we might never know the perf of this channel, back to the drawing board - nim-lang/Nim#12695

* Fix queue, workaround nim-lang/Nim#12695

* Fix fence issue: no strange memory reuse anymore with both Nim and system alloc

* Fix #19 #20: Snmalloc / Pony-lang MPSC queues issues:
- they hold on the last item of a queue (breaking for steal requests)
- they require memory management of the dummy node (snmalloc deletes it and its memory doesn't seem to be reclaimed)
- they never touch the "back" pointer of the queue when dequeuing, meaning if an item was last, dequeuing will still points to it. Pony has an emptiness check via tagged pointer and snmalloc does ???
@jangko
Copy link
Contributor

jangko commented Jul 18, 2020

Atomic*[T] = object 
     when T is Trivial: 
       value: T.atomicType 
     else: 
       nonAtomicValue: T 
       guard: AtomicFlag

If the problem is triggered by value: T.atomicType, then this is a duplicate of #5540, or at least they have same syndrome. The atomicType template is evaluated too early by Nim compiler, it should be delayed until next pass when the Atomic instance is evaluated.

@mratsim
Copy link
Collaborator Author

mratsim commented Jan 21, 2022

After #19422 we now get:

.choosenim/toolchains/nim-#devel/lib/pure/concurrency/atomics.nim(338, 42) Error: undeclared field: 'value' for type atomics.Atomic [type declared in /home/beta/.choosenim/toolchains/nim-#devel/lib/pure/concurrency/atomics.nim(299, 7)]

With line 338

proc store*[T: Trivial](location: var Atomic[T]; desired: T; order: MemoryOrder = moSequentiallyConsistent) {.inline.} =
atomic_store_explicit(addr(location.value), cast[nonAtomicType(T)](desired), order)

And line 299

type
AtomicFlag* {.importc: "atomic_flag", size: 1.} = object
Atomic*[T] = object
when T is Trivial:
# Maps the size of a trivial type to it's internal atomic type
when sizeof(T) == 1: value: AtomicInt8
elif sizeof(T) == 2: value: AtomicInt16
elif sizeof(T) == 4: value: AtomicInt32
elif sizeof(T) == 8: value: AtomicInt64
else:
nonAtomicValue: T
guard: AtomicFlag

@shirleyquirk
Copy link
Contributor

shirleyquirk commented Mar 16, 2023

change nonAtomicValue to value in atomics.nim and both examples compile on devel. still hangs on 1.6.12.

@shirleyquirk
Copy link
Contributor

shirleyquirk commented Mar 17, 2023

i can reduce this to:

type
  Foo[T] = object
    when T is ptr:
      value:T
    else:
      notvalue:T
proc access[A](self:A) = discard
proc store[T:ptr](self: var Foo[T]) = access(self.value)
#proc store[T:not ptr](self: var Foo[T]) = access(self.notvalue)

type
  MyType = ptr object
    next: Foo[MyType]

var f:Foo[MyType]

f.store()

see the above linked issue for a similar bug

Edit: this isn't a completely faithful reduction, as changing 'notvalue' to 'value' doesn't make the compiler hang on 1.6.12

@metagn
Copy link
Collaborator

metagn commented Sep 16, 2023

type
  Foo[T] = object
    when T is ptr:
      value:T
    else:
      notvalue:T
proc access[A](self:A) = discard
proc store[T:ptr](self: Foo[T]) = access(self.value)

import macros
macro tk(x: typedesc) =
  echo getTypeImpl(x)[1].typeKind
template bar(x: typedesc): untyped =
  static:
    echo x
    echo x is ptr
    tk x
  x

type
  MyType = ptr object
    next: Foo[bar(MyType)]

var f:Foo[MyType]

f.store()
ntyForward
MyType
false
Error: undeclared field: 'value' for type Foo

Foo is instantiated before the symbol MyType is known to be a ptr object. Related #1500

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants