Thread overview | |||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
|
July 03, 2015 core.atomics and handling of backward MemoryOrders | ||||
---|---|---|---|---|
| ||||
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. |
July 03, 2015 Re: core.atomics and handling of backward MemoryOrders | ||||
---|---|---|---|---|
| ||||
Posted in reply to Iain Buclaw | 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.
|
July 04, 2015 Re: core.atomics and handling of backward MemoryOrders | ||||
---|---|---|---|---|
| ||||
Posted in reply to rsw0x Attachments:
| 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. |
July 04, 2015 Re: core.atomics and handling of backward MemoryOrders | ||||
---|---|---|---|---|
| ||||
Posted in reply to Iain Buclaw | 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.
|
July 04, 2015 Re: core.atomics and handling of backward MemoryOrders | ||||
---|---|---|---|---|
| ||||
Posted in reply to rsw0x Attachments:
| 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. |
July 04, 2015 Re: core.atomics and handling of backward MemoryOrders | ||||
---|---|---|---|---|
| ||||
Attachments:
| 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. |
July 08, 2015 Re: core.atomics and handling of backward MemoryOrders | ||||
---|---|---|---|---|
| ||||
Posted in reply to Iain Buclaw | 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++.
|
July 08, 2015 Re: core.atomics and handling of backward MemoryOrders | ||||
---|---|---|---|---|
| ||||
Posted in reply to Martin Nowak Attachments:
| 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
|
July 08, 2015 Re: core.atomics and handling of backward MemoryOrders | ||||
---|---|---|---|---|
| ||||
Posted in reply to Iain Buclaw | 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.
|
Copyright © 1999-2021 by the D Language Foundation