Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive
Merged
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
4 changes: 2 additions & 2 deletions changelog/atomic_cas.dd
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Added overloads for `core.atomic.cas` which don't discard their result.
Added overloads for `core.atomic : cas` which don't discard their result
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated style change
Ditto for all other changelogs and template constraints


Existing `core.atomic.cas` functions discard the result, which make it impossible to implement certain operations.
Existing `core.atomic : cas` functions discard the result, which make it impossible to implement certain operations.
A new set of overloads was added which take `ifThis` by pointer and write the result back to the argument.
4 changes: 2 additions & 2 deletions changelog/atomic_exchange.dd
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
Added `core.atomic.atomicExchange`.
Added `core.atomic : atomicExchange`

Added missing `core.atomic.atomicExchange` function to the atomic suite.
Added missing `atomicExchange` function to the atomic suite.
4 changes: 2 additions & 2 deletions changelog/std_string.dd
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Added `core.stdcpp.string`.
Added `core.stdcpp : string`

Added `core.stdcpp.string`, which links against C++ `std::basic_string`
Added `core.stdcpp : string`, which links against C++ `std::basic_string`.

Known issues:

Expand Down
4 changes: 2 additions & 2 deletions changelog/std_vector.dd
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
Added `core.stdcpp.vector`.
Added `core.stdcpp : vector`

Added `core.stdcpp.vector`, which links against C++ `std::vector`
Added `core.stdcpp : vector`, which links against C++ `std::vector`.
17 changes: 13 additions & 4 deletions src/core/atomic.d
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned")
* true if the store occurred, false if not.
*/
bool casWeak(MemoryOrder succ = MemoryOrder.seq,MemoryOrder fail = MemoryOrder.seq,T,V)(T* here, T* ifThis, V writeThis) pure nothrow @nogc @trusted
if (!is(T == shared S, S) && !is(V == shared U, U))
if (!is(T == shared S, S) && !is(V == shared U, U))
Copy link
Contributor

@wilzbach wilzbach Oct 6, 2019

Choose a reason for hiding this comment

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

Before your change this was the official DStyle (https://dlang.org/dstyle).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was inconsistent with the rest of the file... I would always favour internal consistency :/

Copy link
Contributor

@wilzbach wilzbach Oct 8, 2019

Choose a reason for hiding this comment

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

We have a style guide for a reason :/

in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned")
{
// resolve implicit conversions
Expand All @@ -483,7 +483,7 @@ in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned")

/// Ditto
bool casWeak(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(T : V1) || is(shared T : V1)))
in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned")
{
static if (is (V1 == shared U1, U1))
Expand All @@ -508,7 +508,7 @@ in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned")

/// Ditto
bool casWeak(MemoryOrder succ = MemoryOrder.seq,MemoryOrder fail = MemoryOrder.seq,T,V)(shared(T)* here, shared(T)* ifThis, shared(V) writeThis) pure nothrow @nogc @trusted
if (is(T == class))
if (is(T == class))
in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned")
{
return atomicCompareExchangeWeak!(succ, fail)(cast(T*)here, cast(T*)ifThis, cast(V)writeThis);
Expand All @@ -519,11 +519,20 @@ in (atomicPtrIsProperlyAligned(here), "Argument `here` is not properly aligned")
* that all loads and stores before a call to this function are executed before any
* loads and stores after the call.
*/
void atomicFence(MemoryOrder order = MemoryOrder.seq)() nothrow @nogc @safe
void atomicFence(MemoryOrder order = MemoryOrder.seq)() pure nothrow @nogc @safe
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to pause(), please move this into another PR
Ditto for all other pure additions

{
core.internal.atomic.atomicFence!order();
}

/**
* Gives a hint to the processor that the calling thread is in a 'spin-wait' loop,
* allowing to more efficiently allocate resources.
*/
void pause() pure nothrow @nogc @safe
{
core.internal.atomic.pause();
}

/**
* Performs the binary operation 'op' on val using 'mod' as the modifier.
*
Expand Down
31 changes: 29 additions & 2 deletions src/core/internal/atomic.d
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ bool atomicCompareExchangeStrongNoResult(MemoryOrder succ = MemoryOrder.seq, Mem
static assert (false, "Unsupported architecture.");
}

void atomicFence(MemoryOrder order = MemoryOrder.seq)() nothrow @nogc @safe
void atomicFence(MemoryOrder order = MemoryOrder.seq)() pure nothrow @nogc @safe
{
// TODO: `mfence` should only be required for seq_cst operations, but this depends on
// the compiler's backend knowledge to not reorder code inappropriately,
Expand Down Expand Up @@ -633,7 +633,7 @@ void atomicFence(MemoryOrder order = MemoryOrder.seq)() nothrow @nogc @safe
}
else version (D_InlineAsm_X86_64)
{
asm nothrow @nogc @trusted
asm pure nothrow @nogc @trusted
{
naked;
mfence;
Expand All @@ -645,6 +645,33 @@ void atomicFence(MemoryOrder order = MemoryOrder.seq)() nothrow @nogc @safe
}
}

void pause() pure nothrow @nogc @safe
{
version (D_InlineAsm_X86)
{
asm pure nothrow @nogc @trusted
{
naked;
rep; nop;
ret;
}
}
else version (D_InlineAsm_X86_64)
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep the old enum/version declaration instead of duplicating the assembly? Or should there be different implementations according to your comment regarding the pause opcode?

{
asm pure nothrow @nogc @trusted
{
naked;
// pause; // TODO: DMD should add this opcode to its inline asm
rep; nop;
ret;
}
}
else
{
// ARM should `yield`
// other architectures? otherwise some sort of nop...
}
}

private:

Expand Down
18 changes: 1 addition & 17 deletions src/core/internal/spinlock.d
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ shared struct SpinLock
{
import core.time;
if (k < pauseThresh)
return pause();
return core.atomic.pause();
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct if im wrong but this is more than a simple refactoring as the new inline assembly differs:
Old:

rep
nop

New:

naked
rep
nop
ret

else if (k < 32)
return Thread.yield();
Thread.sleep(1.msecs);
Expand All @@ -67,25 +67,9 @@ private:
enum X86 = false;

static if (X86)
{
enum pauseThresh = 16;
void pause()
{
asm @trusted @nogc nothrow
{
// pause instruction
rep;
nop;
}
}
}
else
{
enum pauseThresh = 4;
void pause()
{
}
}

size_t val;
Contention contention;
Expand Down