Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 63 additions & 8 deletions src/core/atomic.d
Original file line number Diff line number Diff line change
Expand Up @@ -264,20 +264,35 @@ 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.
* writeThis = The value to store.
* 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
if (!is(T == shared) && is(T : V1))
bool cas
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird life choices

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I don't like it either, hence why I'm experimenting with: #3095

(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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hard to tell from the diff if you made functional changes or just reformat the existing code...
i'm having to audit this carefully, but i probably don't need to?
if this sort of refactor is fair game, i'll definitely just change this right back next time i touch it ;)

in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned")
{
// resolve implicit conversions
Expand All @@ -294,8 +309,10 @@ 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)))
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)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see there is hidden changes!
what's the logic behind turning the type conversion around?
why is it interesting if here is implicitly convertible to ifThis? here is what's being mutated...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should review commit-by-commit, that will be much easier to see, and you will also see the rationale for it ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I'm sorry! I somehow looked at the inverse of what you're doing.
This is bizarre. Is it my mistake? Weird bug! O_O

So, I wonder if it's actually necessary for V1 to participate in the constraint?
It also seems odd for V1 to participate in the constraint but not V2...
I wonder if this is(V1 : T) || is(V1 : shared T) could be something like is(RemoveShared!V1 : T) ?

in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned")
{
static if (is (V1 == shared U1, U1))
Expand Down Expand Up @@ -1194,4 +1211,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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar.

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
}
}
21 changes: 18 additions & 3 deletions src/core/internal/traits.d
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down