[GHC] #14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Prelude | Version: 8.2.1 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: Incorrect result Unknown/Multiple | at runtime Test Case: | Blocked By: Blocking: | Related Tickets: #12537 Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- The comments in `compiler/prelude/primops.txt.pp` for both operations say "... Implies a full memory barrier." The implementation does not issue any barriers as exemplified by the 32-bit variants as can be seen in the excerpts from `libraries/ghc- prim/cbits/atomic.c`. {{{#!c StgWord hs_atomicread32(StgWord x) { return *(volatile StgWord32 *) x; } }}} and {{{#!c void hs_atomicwrite32(StgWord x, StgWord val) { *(volatile StgWord32 *) x = (StgWord32) val; } }}} The native code generator for X86/amd64 and the LLVM backend do not generate calls to these functions but generate the necessary barrier (`mfence`) directly and thus are not affected by this issue. There are no gcc `__sync_*` intrinsics for the two operations. The new `__atomic_*` intrinisics have the required operations but require gcc 4.7 or later. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * priority: normal => high * milestone: => 8.4.1 Comment: This is quite bad; we should likely just require GCC 4.7 and use the new intrinsics in this case. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:1> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: trommler Type: bug | Status: new Priority: high | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by trommler): * owner: (none) => trommler Comment: I'll prepare a patch. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:2> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: trommler Type: bug | Status: patch Priority: high | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Changes (by trommler): * status: new => patch * differential: => Phab:D4009 -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:3> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: trommler Type: bug | Status: patch Priority: high | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Comment (by Ben Gamari <ben@…>): In [changeset:"bd765f4b1332b3d2a7908de3f9ff1d50da0e0b1d/ghc" bd765f4/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="bd765f4b1332b3d2a7908de3f9ff1d50da0e0b1d" Fix atomicread/write operations In `libraries/ghc-prim/cbits/atomic.c` no barriers were issued for atomic read and write operations. Starting with gcc 4.7 compiler intrinsics are offered. The atomic intrinisics are also available in clang. Use these to implement `hs_atomicread*` and `hs_atomicwrite`. Test Plan: validate on OSX and Windows Reviewers: austin, bgamari, simonmar, hvr, erikd, dfeuer Reviewed By: bgamari Subscribers: dfeuer, rwbarton, thomie GHC Trac Issues: #14244 Differential Revision: https://phabricator.haskell.org/D4009 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:4> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: trommler Type: bug | Status: patch Priority: high | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Comment (by Ben Gamari <ben@…>): In [changeset:"59de290928e6903337f31c1f8107ac8a98ea145d/ghc" 59de2909/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="59de290928e6903337f31c1f8107ac8a98ea145d" Update autoconf test for gcc to require 4.7 and up Fixing #14244 required the newer gcc atomic built-ins that are provided from 4.7 and up. This updates the test to check for minimum gcc version 4.7. The version tests for 3.4 (!), 4.4, and 4.6 are no longer needed and can be removed. This makes the build system simpler. Test Plan: validate Reviewers: austin, bgamari, hvr, simonmar Reviewed By: bgamari Subscribers: rwbarton, thomie, erikd Differential Revision: https://phabricator.haskell.org/D4165 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:5> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: trommler Type: bug | Status: closed Priority: high | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: patch => closed * resolution: => fixed -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:6> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: trommler Type: bug | Status: closed Priority: high | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Comment (by YitzGale): Hmm, by requiring GCC 4.7, this bans GHC from RHEL/CentOS 6, which uses GCC 4.4. That OS will be supported until the end of 2020, and will continue to be used fairly widely in the enterprise until close to then. Can this requirement be avoided at least for x86, amd64, and llvm builds, where it isn't actually needed? -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:7> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: trommler Type: bug | Status: closed Priority: high | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Comment (by trommler): If that helps, GCC 4.7 and up is only a build-time requirement. There are newer versions of GCC available for RHEL and CentOS as part of the Red Hat Developer Toolset for instance. In the case of the LLVM-backend you need a fairly recent (and IIRC an exact) version both at build-time and at run-time. If you are going to install LLVM then you could use clang that comes with it. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:8> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: (none) Type: bug | Status: new Priority: highest | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * owner: trommler => (none) * priority: high => highest * status: closed => new * resolution: fixed => Comment: Reopening to ensure I don't forget to fix this. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:9> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: (none) Type: bug | Status: new Priority: highest | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Comment (by allbery_b): This doesn't consider platforms that use clang as their compiler. And there's been at least one person trying to use icc recently, which iirc used to work. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:10> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: (none) Type: bug | Status: new Priority: highest | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Comment (by allbery_b): uh, I should specify that I mean the configure test doesn't. (I have no idea if icc even has these primitives, or whether they look like gcc's.) -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:11> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: (none) Type: bug | Status: new Priority: highest | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Comment (by bgamari):
uh, I should specify that I mean the configure test doesn't.
What precisely is the problem? As far as I can tell Clang has supported both `__atomic_*` and `__sync_* builtins at least as far back as LLVM 3.4. I'm fairly certain that `__sync_*` are supported by `icc` as I have read that this builtins were stolen by `gcc` from `icc`. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:12> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: (none) Type: bug | Status: patch Priority: highest | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: new => patch -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:13> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: (none) Type: bug | Status: patch Priority: highest | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Comment (by Ben Gamari <ben@…>): In [changeset:"217e4170bdce3df28a667803ce5e619553bfecdd/ghc" 217e417/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="217e4170bdce3df28a667803ce5e619553bfecdd" ghc-prim: Emulate C11 atomics when not available GCC's __sync primitives apparently "usually" imply a full barrier, meaning they can be used to emulate the more precise C11 atomics albeit with a loss of efficiency. This restores compatibility with GCC 4.4. This partially reverts commit 59de290928e6903337f31c1f8107ac8a98ea145d. Test Plan: Validate on Centos Reviewers: hvr, simonmar, trommler Subscribers: rwbarton, thomie, erikd, carter GHC Trac Issues: #14244 Differential Revision: https://phabricator.haskell.org/D4364 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:14> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14244: ghc-prim: hs_atomicread* and hs_atomicwrite* missing barriers -------------------------------------+------------------------------------- Reporter: trommler | Owner: (none) Type: bug | Status: closed Priority: highest | Milestone: 8.4.1 Component: Prelude | Version: 8.2.1 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #12537 | Differential Rev(s): Phab:D4009 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: patch => closed * resolution: => fixed Comment: Merged to `ghc-8.4` as 7a3a7ee938b05a297e4b624cae86e4f0caefb88a. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14244#comment:15> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
participants (1)
-
GHC