opendlang / opend

Boost Software License 1.0
76 stars 17 forks source link

Add C++ like atomics to D #33

Closed IgorDeepakM closed 5 months ago

IgorDeepakM commented 5 months ago

This PR adds C++ like atomics to D similar to std::atomic, https://en.cppreference.com/w/cpp/atomic/atomic. This change will make people who are used to std::atomic feel more like home. Also the C++ atomic class is much nicer to use than the current interface in D, in my opinion.

shared int a = 5;
a.atomicOp!"+="(4);

versus

Atomic!int a = 5;
a += 4;

Also added atomicFetchAnd, atomicFetchOr and AtomicFetchXor in core/internal/atomic.d so that it exposes a system independent interface. Currently the atomic implementation is kind of a mix system dependent and independent in core/atomic.d which should be changed in the future. For example in atomicOp, it uses a compareExchange operation for AND, OR, XOR but this is for x86 only.

Then there is also a stdatomic.d in phobos, which implements AND, OR, XOR with compareExchange for all platforms it seems. This is incorrect as it is mostly x86 that is forced to implement those using compareExchange. Other CPUs use other methods. The operations should use atomicFetchAdd, atomicFetchSub, atomicFetchAnd, atomicFetchOr, atomicFetchXor. Which should be implemented in core/internal/atomic.d. These operations also matches the builtins of LLVM and GCC backends well.

Another question regarding the AND, OR, XOR on x86 is implemented with an atomic load in the beginning.

T atomicFetchAnd(MemoryOrder order = MemoryOrder.seq, T)(T* dest, T value) pure nothrow @nogc @trusted
    if (is(T : ulong))
    {
        T set, get = atomicLoad!(order)(dest); <------------- Do we need this????????????

        do
        {
            set = get & value;
        } while (!atomicCompareExchangeWeak!(MemoryOrder.seq, MemoryOrder.seq)(dest, &get, set));

        return get;
    }

I've looked around on compiled code and I usually only see "normal" mov instruction meaning that it initial load doesn't need to be atomic. It is the xcmpchg instruction that is the actual atomic operation. Can anyone who is prolific in x86 assembly explain what is necessary here.

So there is likely to be a continuation on this.

adamdruppe commented 5 months ago

Looks good enough for me. Will wait to see if someone with more expertise in this area wants to come in with a further review, but if not, I'll merge it... probably Wednesday.

kassane commented 5 months ago

And core.stdc.stdatomic? https://github.com/opendlang/opend/blob/master/druntime/src/core/stdc/stdatomic.d

IgorDeepakM commented 5 months ago

And core.stdc.stdatomic? https://github.com/opendlang/opend/blob/master/druntime/src/core/stdc/stdatomic.d

Yes, I'm aware of it. I will change this to use core/internal/atomic.d in order to use atomicFetchAnd and the like in an upcoming PR. I took the implementation from atomicFetchAnd from this file, which was originally taken from the atomicOp implementation.

denizzzka commented 5 months ago
Atomic!int a = 5;
a += 4;

Why is this PR is not for std.atomic? I think it would be more consistent

Oh, Phobos wasn't forked

denizzzka commented 5 months ago

upd: Phobos is also forked, didn't notice the directory at first sight