Re: [Gc] libatomic_ops CAS ordering constraints correspondence to C11 atomic ones
Hans Boehm <[email protected]> Wed, 4 May 2016 19:11:45 -0700
| Newsgroups | gmane.comp.programming.garbage-collection.boehmgc |
|---|---|
| Message-ID | <CAPUmR1Z7BvCj0eP=Azs7ZmH2_57DSnHQKYFE4YqcmmfURiTZZA@mail.gmail.com> |
--===============0901899727== Content-Type: multipart/alternative; boundary=001a113f0c5a37eaeb05320eddee --001a113f0c5a37eaeb05320eddee Content-Type: text/plain; charset=UTF-8 [Sorry about the slow response.] On Wed, Mar 30, 2016 at 4:08 PM, Ivan Maidanski <ivmai-JGs/[email protected]> wrote: > > Hello Hans, > > I agree about cas_full to be defined as acq_rel | acquire to avoid performance regression on replacement asm with C11 atomics. > But the documentation should be refined and the whole code base should be reviewed to avoid (or minimize) inconsistencies (e.g. the behavior should not depend on AO_PREFER_GENERALIZED presence/absence). > And we definitely do not want stability issues in apps using old AO releases after update to the upcoming one (except for AArch64 which already uses C11 atomics some of them might have too strict memory ordering mapping). Actually I agree that Aarch64 is a problem. We should probably change the documentation to make it clear that for _acquire and _read the ordering is between the load and subsequent operations, except in the nop case, where it orders all prior and all subsequent operations. And similarly for _release and _write. The Aarch64 implementation already weakens it in this way. The alternative is to not use Aarch64 acquire/release operations, and use DMB instead. But that plays poorly with code that does this right. > > > For operations that don't include a read operation, we seem to map _full to something like a release (if there's a write) plus a trailing seq_cst fence. > I see. Please correct the mapping below. > > Let's review rest operations ('=' stands for definition in generalize.h): > > nop: > * read | acquire > * write | release Yes. In hindsight, we should never have used read and write. > * full | acq_rel or seq_cst? should it match __sync_synchronize? It should just be seq_cst, which I hope finally matches sync_synchronize. Nobody should be using the latter. > > load: > * none | relax > * acq | acquire Yes. > * read = load + nop_read Or better yet, acquire. We should be strengthening the read and write stuff to the more general acquire/release versions. It almost never makes a difference and is much less error-prone. > * full = load + nop_full or load + nop_read? I think I would just map it to seq_cst and clarify the spec. > * anything else worth to map directly to C11 atomics (among existing ao orders)? I would map dd_acquire_read to acquire. Ideally it should map to consume, but that's currently broken, and will map to acquire anyway. > > store: > * none | relax > * rel | release > * write = nop_write + none I would again just use release. > * full = release + nop_full or nop_read + release? > * anything else? > > test_and_set: > * none | relax > * acq | acquire > * rel | release Yes. > * full | acq_rel or seq_cst? (note that nop_full = test_and_set_full(&dummy)) I would make it seq_cst. It probably doesn't really matter. We should explicitly define nop_full to be atomic_thread_fence(memory_order_seq_cst). test_and_set_full(&dummy)) actually doesn't do the right thing on ARMv8, where it needs to be "dmb ish". > * anything else? > > fetch_and_add: same as test_and_set? I think so. > > and, or, xor: same as fetch_and_add? > I would change the documentation of _full to _full: The associated operation is ordered with respect to both earlier and later memory ops. If the associated operation is nop, then this orders all earlier memory operations with respect to subsequent ones. AO_store_full or AO_nop_full are the normal ways to force a store to be ordered with respect to a later load. > > Regards, > Ivan --001a113f0c5a37eaeb05320eddee Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr">[Sorry about the slow response.]<div><br>On Wed, Mar 30, 2= 016 at 4:08 PM, Ivan Maidanski <<a href=3D"mailto:ivmai-JGs/[email protected]" target= =3D"_blank">ivmai-JGs/[email protected]</a>> wrote:<br>><br>> Hello Hans,<br>>= ;<br>> I agree about cas_full to be defined as acq_rel | acquire to avoi= d performance regression on replacement asm with C11 atomics.<br>> But t= he documentation should be refined and the whole code base should be review= ed to avoid (or minimize) inconsistencies (e.g. the behavior should not dep= end on AO_PREFER_GENERALIZED presence/absence).<br>> And we definitely d= o not want stability issues in apps using old AO releases after update to t= he upcoming one (except for AArch64 which already uses C11 atomics some of = them might have too strict memory ordering mapping).<div>Actually I agree t= hat Aarch64 is a problem.=C2=A0 We should probably change the documentation= to make it clear that for _acquire and _read the ordering is between the l= oad and subsequent operations, except in the nop case, where it orders all = prior and all subsequent operations.=C2=A0 And similarly for _release and _= write.=C2=A0 The Aarch64 implementation already weakens it in this way.</di= v><div><br></div><div>The alternative is to not use Aarch64 acquire/release= operations, and use DMB instead.=C2=A0 But that plays poorly with code tha= t does this right.</div><div><br>><br>> > For operations that don&= #39;t include a read operation, we seem to map _full to something like a re= lease (if there's a write) plus a trailing seq_cst fence.<br>> I see= . Please correct the mapping below.<br>><br>> Let's review rest o= perations ('=3D' stands for definition in generalize.h):<br>><br= >> nop:<br>> * read | acquire<br>> * write | release<div>Yes.=C2= =A0 In hindsight, we should never have used read and write.</div><div><br>&= gt; * full | acq_rel or seq_cst? should it match __sync_synchronize?</div><= div>It should just be seq_cst, which I hope finally matches sync_synchroniz= e.=C2=A0 Nobody should be using the latter.</div><div><br>><br>> load= :<br>> * none | relax<br>> * acq | acquire</div><div>Yes.</div><div><= br>> * read =3D load + nop_read</div><div>Or better yet, acquire.=C2=A0 = We should be strengthening the read and write stuff to the more general acq= uire/release versions. It almost never makes a difference and is much less = error-prone.</div><div><br>> * full =3D load + nop_full or load + nop_re= ad?</div><div>I think I would just map it to seq_cst and clarify the spec.<= /div><div><br>> * anything else worth to map directly to C11 atomics (am= ong existing ao orders)?</div><div>I would map dd_acquire_read to acquire. = Ideally it should map to consume, but that's currently broken, and will= map to acquire anyway.</div><div><br>><br>> store:<br>> * none | = relax<br>> * rel | release<br>> * write =3D nop_write + none</div><di= v>I would again just use release.</div><div><br>> * full =3D release + n= op_full or nop_read + release?<br>> * anything else?<br>><br>> tes= t_and_set:<br>> * none | relax<br>> * acq | acquire<br>> * rel | r= elease</div><div>Yes.</div><div><br>> * full | acq_rel or seq_cst? (note= that nop_full =3D test_and_set_full(&dummy))</div><div>I would make it= seq_cst.=C2=A0 It probably doesn't really matter.=C2=A0 We should expl= icitly define nop_full to be atomic_thread_fence(memory_order_seq_cst). =C2= =A0test_and_set_full(&dummy)) actually doesn't do the right thing o= n ARMv8, where it needs to be "dmb ish".</div><div><br>> * any= thing else?<br>><br>> fetch_and_add: same as test_and_set?</div><div>= I think so.<br>><br>> and, or, xor: same as fetch_and_add?<br>></d= iv><div>I would change the documentation of _full to</div><div><br></div><d= iv><div>_full: The associated operation is ordered with respect to both ear= lier and later memory ops.</div><div>=C2=A0 =C2=A0 =C2=A0 =C2=A0If the asso= ciated operation is nop, then this orders all earlier memory operations wit= h respect to subsequent ones.</div><div>=C2=A0 =C2=A0 =C2=A0 =C2=A0AO_store= _full or AO_nop_full are the normal ways to force a store</div><div>=C2=A0 = =C2=A0 =C2=A0 =C2=A0to be ordered with respect to a later load.</div>><b= r>> Regards,<br>> Ivan</div></div></div></div> --001a113f0c5a37eaeb05320eddee-- --===============0901899727== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ bdwgc mailing list [email protected] https://lists.opendylan.org/mailman/listinfo/bdwgc --===============0901899727==--