[GDC] Support GDC in core.atomic

[belka-ew]

This is the implementation currently used by GDC.

[dlang-bot]

Thanks for your pull request and interest in making D better, @belka-ew! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "stable + druntime#2596"

[ibuclaw]

Actually, there's a modified version in trunk/9.1 that supports targets like riscv that have libatomic, but no compiler intrinsics. Likewise there's a new fallback path to accommodate the absence of any atomics at all.

Both of which will need to be included in this.

[n8sh]

Does GDC treat the modules gcc.config and gcc.builtins specially?

[ibuclaw]

The former is a template config.d.in that's generated at configure-time. The latter is an empty module that has all builtins injected into it at compile-time, its contents may vary depending on compiler flags.

[belka-ew]

@ibuclaw I updated the code from trunk. Is this what you meant?

[ibuclaw]

@belka-ew, yes, PR looks correct now.

[rainers]

Not sure if it is critical but this looks a bit dangerous if initialization is requested from two threads concurrently.
What platforms match the condition (!GNU_Have_Atomics && !GNU_Have_LibAtomic)? I guess Windows will always have atomics.

[ibuclaw]

I think the only way that could happen is if a C program calls rt_init in two threads concurrently. Using pragma(crt_constructor) is another option, though I don't recall adding that feature to the dmd-cxx branch.

[rainers]

Because there is an atomic operation somewhere called from rt_init? A comment to that respect would be nice in that case.

[ibuclaw]

Hmm, I did write this at the 11th hour, so wasn't strictly thinking too clear about this. Also this was committed before @jpf91 added gcc.gthreads, so I guess we could remove some duplication here.

[ibuclaw]

I think something like this would be more succinct, and doesn't have the same pitfall.

static if (!GNU_Have_Atomics && !GNU_Have_LibAtomic)
{
    import gcc.gthread;

    // Internal static mutex reference.
    private __gthread_mutex_t* _getAtomicMutex() @trusted @nogc nothrow @property
    {
        __gshared static __gthread_mutex_t mutex = GTHREAD_MUTEX_INIT;
        return &mutex;
    }

    // Implements lock/unlock operations.
    private int _atomicMutexLock(__gthread_mutex_t* mutex) @trusted @nogc nothrow
    {
        return __gthread_mutex_lock(mutex);
    }

    private int _atomicMutexUnlock(__gthread_mutex_t* mutex) @trusted @nogc nothrow
    {
        return __gthread_mutex_unlock(mutex);
    }

    // Fake the purity of the functions so that they can be used in pure/nothrow/@safe code.
    pragma(mangle, _getAtomicMutex.mangleof)
    private __gthread_mutex_t* getAtomicMutex() pure @trusted @nogc nothrow @property;

    pragma(mangle, _atomicMutexLock.mangleof)
    private int atomicMutexLock(__gthread_mutex_t*) pure @trusted @nogc nothrow;

    pragma(mangle, _atomicMutexUnlock.mangleof)
    private int atomicMutexUnlock(__gthread_mutex_t*) pure @trusted @nogc nothrow;
}

Once __gthread_mutex_t and functions have been added to gcc.gthread, that is.

[jpf91]

This should work. However, I don't really like that we duplicate core.sync.mutex here. Is this only required because core.sync.mutex is not pure? If so, can we change it to be pure?

[n8sh]

BTW what is the motivation for this PR? Is the goal to upstream all druntime & phobos changes in LDC & GDC?

[n8sh]

Can GDC use __atomic_add_fetch instead of a compare and swap loop for "+=" or is that not available?

[belka-ew]

BTW what is the motivation for this PR? Is the goal to upstream all druntime & phobos changes in LDC & GDC?

Yes. druntime contains already compiler and platform dependent code. To be honest I don't see a better way to make druntime more extendable, so that some functionality can be implemented in the compiler and made available for the drutnime.
Currently updating the druntime is a pain, I have to look what files have been changed, apply changes, resolve conflicts, look if everything has been updated...

[ibuclaw]

BTW what is the motivation for this PR? Is the goal to upstream all druntime & phobos changes in LDC & GDC?

Druntime has always been lenient when it comes to compiler specific things in common/shared modules (core.cpuid, core.stdc.stdlib, rt.qsort).

Phobos should not have any compiler specific code. The only exception to the rule is std.math, but I think that the right way to go there is to turn it into a package, similar to how bigint is split into asm and non-asm parts.

[belka-ew]

Phobos should not have any compiler specific code. The only exception to the rule is std.math, but I think that the right way to go there is to turn it into a package, similar to how bigint is split into asm and non-asm parts.

Or move asm parts as simple functions into druntime and call them from phobos.

[belka-ew]

Druntime has always been lenient when it comes to compiler specific things in common/shared modules (core.cpuid, core.stdc.stdlib, rt.qsort).

stdc.stdarg is the next candidate.

[n8sh]

Please either use __atomic_add_fetch etc. in atomicOp or briefly explain the rationale for using a compare-and-swap loop instead.

[TurkeyMan]

@ibuclaw This PR will completely change your solution here: #2739
I'm trying to separate the platform specific parts from the rest of the code, the functions int core.internal.atomic should be a whole lot easier to substitute with intrinsics.