www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - core.atomics and handling of backward MemoryOrders

reply "Iain Buclaw" <ibuclaw gdcproject.org> writes:
Hi,

I'm currently re-writing core.atomics for GDC to switch from the 
old GCC __sync_* builtins to the new (more compatible with how 
core.atomics is supposed to function) __atomic_* builtins.

https://github.com/D-Programming-GDC/GDC/pull/111

One thing I've noticed in my reading of this, is that the 
following are accepted as valid, but makes no sense.

---
atomicStore!(MemoryOrder.acq)(var, val);

var = atomicLoad!(MemoryOrder.rel)(val);
---

I'd like to suggest that it should be considered completely valid 
for both cases to throw a compilation error (using static 
assert).  However I'd like the core druntime team to be on board 
with this.

Regards
Iain.
Jul 03 2015
parent reply "rsw0x" <anonymous anonymous.com> writes:
On Friday, 3 July 2015 at 17:51:17 UTC, Iain Buclaw wrote:
 Hi,

 I'm currently re-writing core.atomics for GDC to switch from 
 the old GCC __sync_* builtins to the new (more compatible with 
 how core.atomics is supposed to function) __atomic_* builtins.

 https://github.com/D-Programming-GDC/GDC/pull/111

 One thing I've noticed in my reading of this, is that the 
 following are accepted as valid, but makes no sense.

 ---
 atomicStore!(MemoryOrder.acq)(var, val);

 var = atomicLoad!(MemoryOrder.rel)(val);
 ---

 I'd like to suggest that it should be considered completely 
 valid for both cases to throw a compilation error (using static 
 assert).  However I'd like the core druntime team to be on 
 board with this.

 Regards
 Iain.
IIRC these flat out error on LDC, I meant to submit a bug report but I forgot until I read this.
Jul 03 2015
parent reply Iain Buclaw via Digitalmars-d <digitalmars-d puremagic.com> writes:
On 4 Jul 2015 00:50, "rsw0x via Digitalmars-d" <digitalmars-d puremagic.com>
wrote:
 On Friday, 3 July 2015 at 17:51:17 UTC, Iain Buclaw wrote:
 Hi,

 I'm currently re-writing core.atomics for GDC to switch from the old GCC
__sync_* builtins to the new (more compatible with how core.atomics is supposed to function) __atomic_* builtins.
 https://github.com/D-Programming-GDC/GDC/pull/111

 One thing I've noticed in my reading of this, is that the following are
accepted as valid, but makes no sense.
 ---
 atomicStore!(MemoryOrder.acq)(var, val);

 var = atomicLoad!(MemoryOrder.rel)(val);
 ---

 I'd like to suggest that it should be considered completely valid for
both cases to throw a compilation error (using static assert). However I'd like the core druntime team to be on board with this.
 Regards
 Iain.
IIRC these flat out error on LDC, I meant to submit a bug report but I
forgot until I read this. Good to know, as I understand it, there's currently the following behaviour in dmd runtime. atomicLoad!(Memoryorder.seq) -> issues a lock+exchange. atomicLoad!(Memoryorder.acq) -> lock-free (only because of x86 guarantees). atomicLoad!(Memoryorder.rel) -> issues a lock+exchange. atomicLoad!(Memoryorder.raw) -> no lock. atomicStore!(Memoryorder.seq) -> issues a lock+exchange. atomicStore!(Memoryorder.acq) -> issues a lock+exchange. atomicStore!(Memoryorder.rel) -> lock-free (only because of x86 guarantees). atomicStore!(Memoryorder.raw) -> no lock. Is it fine to consider the following (for optimisation purposes, of course :) atomicLoad!(Memoryorder.seq) -> Can be lock-free (only because of x86 guarantees), but guaranteed to do the right thing otherwise. atomicLoad!(Memoryorder.acq) -> Can be lock-free (only because of x86 guarantees), but guaranteed to do the right thing otherwise. atomicLoad!(Memoryorder.rel) -> static assert. atomicLoad!(Memoryorder.raw) -> no lock. atomicStore!(Memoryorder.seq) -> Can be lock-free (only because of x86 guarantees), but guaranteed to do the right thing otherwise. atomicStore!(Memoryorder.acq) -> static assert. atomicStore!(Memoryorder.rel) -> Can be lock-free (only because of x86 guarantees), but guaranteed to do the right thing otherwise. atomicStore!(Memoryorder.raw) -> no lock. Iain.
Jul 04 2015
parent reply "rsw0x" <anonymous anonymous.com> writes:
On Saturday, 4 July 2015 at 07:16:09 UTC, Iain Buclaw wrote:
 On 4 Jul 2015 00:50, "rsw0x via Digitalmars-d" 
 <digitalmars-d puremagic.com> wrote:
 [...]
__sync_* builtins to the new (more compatible with how core.atomics is supposed to function) __atomic_* builtins.
 [...]
accepted as valid, but makes no sense.
 [...]
both cases to throw a compilation error (using static assert). However I'd like the core druntime team to be on board with this.
 [...]
forgot until I read this. [...]
This is how it's currently implemented in C++ as of C++14, correct? acquire semantics on a write and release semantics on a load make no sense, so this probably should be changed.
Jul 04 2015
next sibling parent Iain Buclaw via Digitalmars-d <digitalmars-d puremagic.com> writes:
On 4 July 2015 at 10:39, rsw0x via Digitalmars-d <
digitalmars-d puremagic.com> wrote:

 On Saturday, 4 July 2015 at 07:16:09 UTC, Iain Buclaw wrote:

 On 4 Jul 2015 00:50, "rsw0x via Digitalmars-d" <
 digitalmars-d puremagic.com> wrote:

 [...]

 __sync_* builtins to the new (more compatible with how core.atomics is
supposed to function) __atomic_* builtins.
 [...]

 accepted as valid, but makes no sense.
 [...]

 both cases to throw a compilation error (using static assert).  However
I'd like the core druntime team to be on board with this.
 [...]
forgot until I read this. [...]
This is how it's currently implemented in C++ as of C++14, correct? acquire semantics on a write and release semantics on a load make no sense, so this probably should be changed.
Yes, that is correct. I think closely matching the behaviour of C++14 is the safe option given that this module shares a lot in common with std::atomic. std::atomic::atomic_compare_exchange_strong -> core.atomic.cas std::atomic::memory_order -> core.atomic.MemoryOrder std::atomic::atomic_load -> core.atomic.atomicLoad std::atomic::atomic_store -> core.atomic.atomicStore std::atomic::atomic_thread_fence -> core.atomic.atomicFence Iain.
Jul 04 2015
prev sibling parent reply Iain Buclaw via Digitalmars-d <digitalmars-d puremagic.com> writes:
On 4 Jul 2015 11:12, "Iain Buclaw" <ibuclaw gdcproject.org> wrote:
 On 4 July 2015 at 10:39, rsw0x via Digitalmars-d <
digitalmars-d puremagic.com> wrote:
 On Saturday, 4 July 2015 at 07:16:09 UTC, Iain Buclaw wrote:
 On 4 Jul 2015 00:50, "rsw0x via Digitalmars-d" <
digitalmars-d puremagic.com> wrote:
 [...]
__sync_* builtins to the new (more compatible with how core.atomics is
supposed to function) __atomic_* builtins.
 [...]
accepted as valid, but makes no sense.
 [...]
both cases to throw a compilation error (using static assert). However
I'd like the core druntime team to be on board with this.
 [...]
forgot until I read this. [...]
This is how it's currently implemented in C++ as of C++14, correct? acquire semantics on a write and release semantics on a load make no
sense, so this probably should be changed.
 Yes, that is correct.  I think closely matching the behaviour of C++14 is
the safe option given that this module shares a lot in common with std::atomic.
 std::atomic::atomic_compare_exchange_strong -> core.atomic.cas
 std::atomic::memory_order -> core.atomic.MemoryOrder
 std::atomic::atomic_load -> core.atomic.atomicLoad
 std::atomic::atomic_store -> core.atomic.atomicStore
 std::atomic::atomic_thread_fence -> core.atomic.atomicFence

 Iain.
OK, I've added static asserts to my PR. Iain.
Jul 04 2015
parent reply "Martin Nowak" <code dawg.eu> writes:
On Saturday, 4 July 2015 at 10:47:17 UTC, Iain Buclaw wrote:
 both cases to throw a compilation error (using static 
 assert).  However
I'd like the core druntime team to be on board with this.
It makes sense to disallow them, but I wonder why they are allowed in C++.
Jul 08 2015
parent reply Iain Buclaw via Digitalmars-d <digitalmars-d puremagic.com> writes:
On 8 July 2015 at 10:46, Martin Nowak via Digitalmars-d <
digitalmars-d puremagic.com> wrote:

 On Saturday, 4 July 2015 at 10:47:17 UTC, Iain Buclaw wrote:

 both cases to throw a compilation error (using static assert).  However

 I'd like the core druntime team to be on board with this.
It makes sense to disallow them, but I wonder why they are allowed in C++.
They *aren't* allowed in C++. GCC will give you a compiler warning if you attempt it, and stdc++ should invoke a cxx_assert on it's use. Iain
Jul 08 2015
parent "Martin Nowak" <code dawg.eu> writes:
On Wednesday, 8 July 2015 at 13:58:24 UTC, Iain Buclaw wrote:
 GCC will give you a compiler warning if you attempt it, and 
 stdc++ should invoke a cxx_assert on it's use.
I was under the impression that I died them once with no effect (might have been clang). In any case, feel free to go forward with a static assert check.
Jul 08 2015