public inbox for gcc-bugs@sourceware.org help / color / mirror / Atom feed
* [Bug libgomp/45025] Memory ordering issues with libgomp critical sections and __sync [not found] <bug-45025-4@http.gcc.gnu.org/bugzilla/> @ 2012-02-03 2:50 ` pinskia at gcc dot gnu.org 2024-04-09 1:36 ` pinskia at gcc dot gnu.org 1 sibling, 0 replies; 6+ messages in thread From: pinskia at gcc dot gnu.org @ 2012-02-03 2:50 UTC (permalink / raw) To: gcc-bugs http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45025 --- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-02-03 02:49:44 UTC --- I almost think this has been fixed now on the trunk. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug libgomp/45025] Memory ordering issues with libgomp critical sections and __sync [not found] <bug-45025-4@http.gcc.gnu.org/bugzilla/> 2012-02-03 2:50 ` [Bug libgomp/45025] Memory ordering issues with libgomp critical sections and __sync pinskia at gcc dot gnu.org @ 2024-04-09 1:36 ` pinskia at gcc dot gnu.org 1 sibling, 0 replies; 6+ messages in thread From: pinskia at gcc dot gnu.org @ 2024-04-09 1:36 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=45025 Andrew Pinski <pinskia at gcc dot gnu.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED Target Milestone|--- |4.7.0 --- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> --- This was fixed by r0-113642-gb40c885f183cb7 which added the C11 atomics barrier. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug libgomp/45025] New: Memory ordering issues with libgomp critical sections and __sync @ 2010-07-21 18:43 Hans dot Boehm at hp dot com 2010-07-21 22:39 ` [Bug libgomp/45025] " sje at cup dot hp dot com ` (3 more replies) 0 siblings, 4 replies; 6+ messages in thread From: Hans dot Boehm at hp dot com @ 2010-07-21 18:43 UTC (permalink / raw) To: gcc-bugs This is really a continuation of bug 42869, but that title mentions Itanium, and this is not Itanium-specific. Gomp_mutex_lock and as a result OpenMP critical sections and locks do not implement sane memory ordering semantics. This is largely due to __sync implementation problems. I looked at Itanium and PowerPC, but other weakly ordered architectures are probably similar. I was trying to understand the memory ordering semantics of OpenMP critical sections with gcc, since the 3.0 OpenMP spec requires full fence semantics, which seems to be overkill. I concluded that the current state of affairs in 4.4.4 doesn't make much sense, independent of the spec interpretation. Specific issues: 1. The Itanium version of gomp_mutex_lock still has the fence on the wrong side of the cmpxchg4.rel. This is a __sync_bool_compare_and_swap problem, and was fixed in the trunk (bug 42869, thanks Steve!) but still appears to be wrong in the 4.4 series. I think this needs a backport to 4.4, since we may effectively be generating critical sections that don't guarantee mutual exclusion. (I don't know whether current hardware provides stronger properties that mitigate this, but ...) 2. The PowerPC & Itanium version of gomp_mutex_lock explicitly calls __sync_bool_compare_and_swap. Nonetheless the PowerPC code ends up generating a code sequence along the lines of lwsync; lwarx; stwcx.; bne; isync As far as I can determine, this does not order a preceding store with respect to a later load. (It uses lwsync, not sync.) It's unclear whether this is a problem with respect to the OpenMP spec. (Technically it is for 3.0, but 3.1 was reworded, on my suggestion, to allow this, since it's a useless restriction.) But I think it clearly fails to satisfy the __sync_bool_compare_and_swap spec, which claims this should be a "full barrier". More generally, gomp_mutex_lock() appears inconsistent about whether it satisfies the OpenMP 3.0 and earlier rules that it include a flush or full barrier. Modulo the earlier bug, the Itanium versions do. The PowerPC version does for unlock, but not lock. (And given that it doesn't, I'm not sure there's a point behind the lwsync in the lock sequence either.) Since this choice has a significant performance impact on both architectures, this seems to be a way to get the worst of all possible worlds; we get both weak guarantees and, in many cases, bad performance. I would recommend following the OpenMP3.1 draft spec, and insisting on only acquire/release semantics for gomp_mutex_(un)lock. -- Summary: Memory ordering issues with libgomp critical sections and __sync Product: gcc Version: 4.4.4 Status: UNCONFIRMED Severity: normal Priority: P3 Component: libgomp AssignedTo: unassigned at gcc dot gnu dot org ReportedBy: Hans dot Boehm at hp dot com http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45025 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug libgomp/45025] Memory ordering issues with libgomp critical sections and __sync 2010-07-21 18:43 [Bug libgomp/45025] New: " Hans dot Boehm at hp dot com @ 2010-07-21 22:39 ` sje at cup dot hp dot com 2010-07-22 7:59 ` jakub at gcc dot gnu dot org ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: sje at cup dot hp dot com @ 2010-07-21 22:39 UTC (permalink / raw) To: gcc-bugs ------- Comment #1 from sje at cup dot hp dot com 2010-07-21 22:39 ------- I backported the patch for PR 42869 to the 4.4 branch to fix Itanium. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45025 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug libgomp/45025] Memory ordering issues with libgomp critical sections and __sync 2010-07-21 18:43 [Bug libgomp/45025] New: " Hans dot Boehm at hp dot com 2010-07-21 22:39 ` [Bug libgomp/45025] " sje at cup dot hp dot com @ 2010-07-22 7:59 ` jakub at gcc dot gnu dot org 2010-07-22 18:03 ` Hans dot Boehm at hp dot com 2010-07-22 18:41 ` jason at gcc dot gnu dot org 3 siblings, 0 replies; 6+ messages in thread From: jakub at gcc dot gnu dot org @ 2010-07-22 7:59 UTC (permalink / raw) To: gcc-bugs ------- Comment #2 from jakub at gcc dot gnu dot org 2010-07-22 07:59 ------- So, 1. is fixed. If __sync_*compare_and_swap on PowerPC doesn't act as full barrier, that would be a target bug, not libgomp bug. Unless separate __sync_*_acq/__sync_*_rel builtins are added, I'm afraid the only option for libgomp would be on the weakly ordered targets to use inline asm instead, which wouldn't be very pretty. -- jakub at gcc dot gnu dot org changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rth at gcc dot gnu dot org http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45025 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug libgomp/45025] Memory ordering issues with libgomp critical sections and __sync 2010-07-21 18:43 [Bug libgomp/45025] New: " Hans dot Boehm at hp dot com 2010-07-21 22:39 ` [Bug libgomp/45025] " sje at cup dot hp dot com 2010-07-22 7:59 ` jakub at gcc dot gnu dot org @ 2010-07-22 18:03 ` Hans dot Boehm at hp dot com 2010-07-22 18:41 ` jason at gcc dot gnu dot org 3 siblings, 0 replies; 6+ messages in thread From: Hans dot Boehm at hp dot com @ 2010-07-22 18:03 UTC (permalink / raw) To: gcc-bugs ------- Comment #3 from Hans dot Boehm at hp dot com 2010-07-22 18:03 ------- Is there a plan for more complete C++0x/C1x atomics support? Something eventually needs to implement the memory_order_acquire/memory_order_release versions of these primitives? Nptl also seems to be in need of these. I agree that it would be unfortunate to use inline assembly code here at this point in time. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45025 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug libgomp/45025] Memory ordering issues with libgomp critical sections and __sync 2010-07-21 18:43 [Bug libgomp/45025] New: " Hans dot Boehm at hp dot com ` (2 preceding siblings ...) 2010-07-22 18:03 ` Hans dot Boehm at hp dot com @ 2010-07-22 18:41 ` jason at gcc dot gnu dot org 3 siblings, 0 replies; 6+ messages in thread From: jason at gcc dot gnu dot org @ 2010-07-22 18:41 UTC (permalink / raw) To: gcc-bugs ------- Comment #4 from jason at gcc dot gnu dot org 2010-07-22 18:41 ------- (In reply to comment #3) > Is there a plan for more complete C++0x/C1x atomics support? http://gcc.gnu.org/wiki/Atomic -- jason at gcc dot gnu dot org changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |amacleod at redhat dot com http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45025 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-09 1:36 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <bug-45025-4@http.gcc.gnu.org/bugzilla/> 2012-02-03 2:50 ` [Bug libgomp/45025] Memory ordering issues with libgomp critical sections and __sync pinskia at gcc dot gnu.org 2024-04-09 1:36 ` pinskia at gcc dot gnu.org 2010-07-21 18:43 [Bug libgomp/45025] New: " Hans dot Boehm at hp dot com 2010-07-21 22:39 ` [Bug libgomp/45025] " sje at cup dot hp dot com 2010-07-22 7:59 ` jakub at gcc dot gnu dot org 2010-07-22 18:03 ` Hans dot Boehm at hp dot com 2010-07-22 18:41 ` jason at gcc dot gnu dot org
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).