https://issues.dlang.org/show_bug.cgi?id=12891

Jonathan Dunlap <jadit2@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jadit2@gmail.com

--- Comment #1 from Jonathan Dunlap <jadit2@gmail.com> ---
I might take a look at this. Would a lock:xadd be appropriate for this atomicAdd?

--

https://issues.dlang.org/show_bug.cgi?id=12891

Martin Nowak <code@dawg.eu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dsimcha@yahoo.com

--- Comment #2 from Martin Nowak <code@dawg.eu> ---
*** Issue 4769 has been marked as a duplicate of this issue. ***

--

https://issues.dlang.org/show_bug.cgi?id=12891

--- Comment #3 from Martin Nowak <code@dawg.eu> ---
Most likely there is no performance difference between "lock xadd [ptr], 1" and "lock inc [ptr]", the increment instruction takes a byte less.

You can ask g++/clang++ for good ideas.

----
#include <atomic>

std::atomic<uint8_t> ubyte;
std::atomic<uint16_t> ushort;
std::atomic<uint32_t> uint;
std::atomic<uint64_t> ulong;

int main() {
  ++ubyte;
  ++ushort;
  ++uint;
  ++ulong;
}
----

It seems that we should rather implement the more generic fetch_add and fetch_sub though. http://en.cppreference.com/w/cpp/atomic/atomic/fetch_add http://en.cppreference.com/w/cpp/atomic/atomic/fetch_sub

--

https://issues.dlang.org/show_bug.cgi?id=12891

Martin Nowak <code@dawg.eu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|add atomicInc and atomicDec |add atomicFetchAdd and
                   |to core.atomic              |atomicFetchSub to
                   |                            |core.atomic

--

https://issues.dlang.org/show_bug.cgi?id=12891

--- Comment #4 from Jonathan Dunlap <jadit2@gmail.com> ---
I'm having an issue adding an asm block for the fast increment by operation. It's been awhile since I've used asm productively so I'm probably missing something simple.

HeadUnshared!(T) atomicOp(string op, T, V1)( ref shared T val, V1 mod ) pure
nothrow @nogc
        if( __traits( compiles, mixin( "*cast(T*)&val" ~ op ~ "mod" ) ) )
[....]
get = set = atomicLoad!(MemoryOrder.raw)( val );

auto setptr = &set;
static if(op == "+=") {
  // qword ptr
  asm pure nothrow @nogc
  {
    mov EAX, mod;    //:678
    mov EDX, setptr; //:679
    lock;
    xadd [EDX], EAX;
  }
}

Error:
src/core/atomic.d(678): Error: bad type/size of operands 'mov'
src/core/atomic.d(679): Error: bad type/size of operands 'mov'

--

https://issues.dlang.org/show_bug.cgi?id=12891

--- Comment #5 from Martin Nowak <code@dawg.eu> ---
(In reply to Jonathan Dunlap from comment #4)
Depending on the size of mod you need to use a smaller register.

static if (V1.sizeof == 1) asm { mov AL, mod; }
static if (V1.sizeof == 2) asm { mov AX, mod; }
static if (V1.sizeof == 4) asm { mov EAX, mod; }
static if (V1.sizeof == 8) asm { mov RAX, mod; }

Likewise you need to load the pointer (ref) into a fitting register.

mov EDX, val; // x86
mov RDX, val; // x86_64

--

https://issues.dlang.org/show_bug.cgi?id=12891

--- Comment #6 from Martin Nowak <code@dawg.eu> ---
Created attachment 1508
  --> https://issues.dlang.org/attachment.cgi?id=1508&action=edit
atomic fetch add for x64

Implementation of atomicOp!"+=" (fetch_add) for X86_64.

--

https://issues.dlang.org/show_bug.cgi?id=12891

--- Comment #7 from Martin Nowak <code@dawg.eu> ---
See attachment above or this gist https://gist.github.com/MartinNowak/5111611ddc476eb49298 for a fetch_add implementation on X86_64.

--

https://issues.dlang.org/show_bug.cgi?id=12891

--- Comment #8 from Jonathan Dunlap <jadit2@gmail.com> ---
Thanks Martin for the gist sample, I've started to integrate in this branch: https://github.com/jadbox/druntime/tree/fetchmod

Currently I'm having an issue where a static if check against __traits(isSame,T,V1) is always resulting to false, even though it seems T and V1 are the same in the unittests.

--

https://issues.dlang.org/show_bug.cgi?id=12891

Jonathan Dunlap <jadit2@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #9 from Jonathan Dunlap <jadit2@gmail.com> ---
I resolved the issue by replacing the "__traits(isSame, T, V1)" with "is(T ==
V1)".

Okay my branch below seems to pass all the x64 unittests I've made for this. It should also work for x86, but I haven't tested yet. Would someone want to benchmark this?

--