From 963f0b675803b47107f044fe1c3b1e2624a5125a Mon Sep 17 00:00:00 2001 From: Geod24 Date: Mon, 4 May 2020 06:46:07 +0900 Subject: [PATCH 1/3] Recognize immutable in core.internal.traits : hasUnsharedIndirections Due to the lack of recognition for immutable, it wasn't possible to do certain atomic operations, e.g. storing an immutable pointer to a struct into a shared pointer to an immutable struct. --- src/core/internal/traits.d | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/core/internal/traits.d b/src/core/internal/traits.d index 990b95c85d..2dde425589 100644 --- a/src/core/internal/traits.d +++ b/src/core/internal/traits.d @@ -363,22 +363,37 @@ template hasIndirections(T) template hasUnsharedIndirections(T) { - static if (is(T == struct) || is(T == union)) + static if (is(T == immutable)) + enum hasUnsharedIndirections = false; + else static if (is(T == struct) || is(T == union)) enum hasUnsharedIndirections = anySatisfy!(.hasUnsharedIndirections, Fields!T); else static if (is(T : E[N], E, size_t N)) enum hasUnsharedIndirections = is(E == void) ? false : hasUnsharedIndirections!E; else static if (isFunctionPointer!T) enum hasUnsharedIndirections = false; else static if (isPointer!T) - enum hasUnsharedIndirections = !is(T : shared(U)*, U); + enum hasUnsharedIndirections = !is(T : shared(U)*, U) && !is(T : immutable(U)*, U); else static if (isDynamicArray!T) - enum hasUnsharedIndirections = !is(T : shared(V)[], V); + enum hasUnsharedIndirections = !is(T : shared(V)[], V) && !is(T : immutable(V)[], V); else static if (is(T == class) || is(T == interface)) enum hasUnsharedIndirections = !is(T : shared(W), W); else enum hasUnsharedIndirections = isDelegate!T || __traits(isAssociativeArray, T); // TODO: how to handle these? } +unittest +{ + static struct Foo { shared(int)* val; } + + static assert(!hasUnsharedIndirections!(immutable(char)*)); + static assert(!hasUnsharedIndirections!(string)); + + static assert(!hasUnsharedIndirections!(Foo)); + static assert( hasUnsharedIndirections!(Foo*)); + static assert(!hasUnsharedIndirections!(shared(Foo)*)); + static assert(!hasUnsharedIndirections!(immutable(Foo)*)); +} + enum bool isAggregateType(T) = is(T == struct) || is(T == union) || is(T == class) || is(T == interface); From 3a1fa03bbc8803a2083ec126e5d7de841cdd3642 Mon Sep 17 00:00:00 2001 From: Geod24 Date: Mon, 4 May 2020 07:02:48 +0900 Subject: [PATCH 2/3] Fix backward template constraints on `core.atomic.cas` In the case of `cas`, the first argument is a lvalue, while the other two can be rvalue. The template constraint was checking whether the lvalue argument type could be coerced to the rvalue argument type, however that is backward compared to how things are usually done in the language. We want rvalue to coerce to rvalue, never the other way around. --- src/core/atomic.d | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/core/atomic.d b/src/core/atomic.d index 512cf6563a..26796480dd 100644 --- a/src/core/atomic.d +++ b/src/core/atomic.d @@ -277,7 +277,7 @@ in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned") * true if the store occurred, false if not. */ bool cas(MemoryOrder succ = MemoryOrder.seq,MemoryOrder fail = MemoryOrder.seq,T,V1,V2)(T* here, V1 ifThis, V2 writeThis) pure nothrow @nogc @trusted - if (!is(T == shared) && is(T : V1)) + if (!is(T == shared) && is(V1 : T)) in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned") { // resolve implicit conversions @@ -295,7 +295,7 @@ in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned") /// Ditto bool cas(MemoryOrder succ = MemoryOrder.seq,MemoryOrder fail = MemoryOrder.seq,T,V1,V2)(shared(T)* here, V1 ifThis, V2 writeThis) pure nothrow @nogc @trusted - if (!is(T == class) && (is(T : V1) || is(shared T : V1))) + if (!is(T == class) && (is(V1 : T) || is(V1 : shared T))) in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned") { static if (is (V1 == shared U1, U1)) @@ -1194,4 +1194,42 @@ version (CoreUnittest) shared NoIndirections n; static assert(is(typeof(atomicLoad(n)) == NoIndirections)); } + + /* + * Used to trigger: + * --- + * Error: template core.atomic.cas cannot deduce function from argument types + * !()(shared(immutable(Image)*)*, typeof(null), immutable(Image*)), candidates are: + * + * cas(MemoryOrder succ = MemoryOrder.seq, MemoryOrder fail = MemoryOrder.seq, T, V1, V2) + * (T* here, V1 ifThis, V2 writeThis) + * + */ + pure nothrow @safe unittest + { + // A structure that is not atomically initialization (e.g. open files) + static struct Image { void* ptr; } + + // In practice, a global value + shared immutable(Image)* cache; + // Manifest constant for code clarity + immutable Sentinel = () @trusted { return cast(immutable Image*) size_t.max; }(); + + immutable(Image)* local = atomicLoad(cache); + // Two steps initialization + if (local is null && cas(&cache, null, Sentinel)) + { + local = new immutable(Image)(null); + atomicStore(cache, local); + } + else + { + while (local is Sentinel) + { + pause(); + local = atomicLoad(cache); + } + } + // Use local + } } From ea21cd66851fe0a8e367e41bd76b893c454fb6fb Mon Sep 17 00:00:00 2001 From: Geod24 Date: Mon, 4 May 2020 07:27:36 +0900 Subject: [PATCH 3/3] Improve documentation and formatting for `core.atomic.cas` --- src/core/atomic.d | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/core/atomic.d b/src/core/atomic.d index 26796480dd..e286b2eb07 100644 --- a/src/core/atomic.d +++ b/src/core/atomic.d @@ -264,9 +264,19 @@ in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned") } /** - * Stores 'writeThis' to the memory referenced by 'here' if the value - * referenced by 'here' is equal to 'ifThis'. This operation is both - * lock-free and atomic. + * Performs the atomic operation known as "compare-and-swap" (CAS). + * + * Compare-and-swap is essentially the following operation: + * --- + * if (*here == ifThis) + * { + * *here = writeThis; + * return true; + * } + * else + * return false; + * --- + * This operation is both lock-free and atomic. * * Params: * here = The address of the destination variable. @@ -274,9 +284,14 @@ in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned") * ifThis = The comparison value. * * Returns: - * true if the store occurred, false if not. + * Whether the swap occured or not. + * + * See_Also: + * https://en.wikipedia.org/wiki/Compare-and-swap */ -bool cas(MemoryOrder succ = MemoryOrder.seq,MemoryOrder fail = MemoryOrder.seq,T,V1,V2)(T* here, V1 ifThis, V2 writeThis) pure nothrow @nogc @trusted +bool cas + (MemoryOrder succ = MemoryOrder.seq, MemoryOrder fail = MemoryOrder.seq, T, V1, V2) + (T* here, V1 ifThis, V2 writeThis) pure nothrow @nogc @trusted if (!is(T == shared) && is(V1 : T)) in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned") { @@ -294,7 +309,9 @@ in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned") } /// Ditto -bool cas(MemoryOrder succ = MemoryOrder.seq,MemoryOrder fail = MemoryOrder.seq,T,V1,V2)(shared(T)* here, V1 ifThis, V2 writeThis) pure nothrow @nogc @trusted +bool cas + (MemoryOrder succ = MemoryOrder.seq, MemoryOrder fail = MemoryOrder.seq, T, V1, V2) + (shared(T)* here, V1 ifThis, V2 writeThis) pure nothrow @nogc @trusted if (!is(T == class) && (is(V1 : T) || is(V1 : shared T))) in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned") {