{"action":"created","issue":{"url":"https://api.github.com/repos/rrnewton/haskell-lockfree/issues/41","labels_url":"https://api.github.com/repos/rrnewton/haskell-lockfree/issues/41/labels{/name}","comments_url":"https://api.github.com/repos/rrnewton/haskell-lockfree/issues/41/comments","events_url":"https://api.github.com/repos/rrnewton/haskell-lockfree/issues/41/events","html_url":"https://github.com/rrnewton/haskell-lockfree/issues/41","id":53274979,"number":41,"title":"[atomic-primops] Change in `fetchAddIntArray#` in GHC 7.10","user":{"login":"rrnewton","id":274582,"avatar_url":"https://avatars.githubusercontent.com/u/274582?v=3","gravatar_id":"","url":"https://api.github.com/users/rrnewton","html_url":"https://github.com/rrnewton","followers_url":"https://api.github.com/users/rrnewton/followers","following_url":"https://api.github.com/users/rrnewton/following{/other_user}","gists_url":"https://api.github.com/users/rrnewton/gists{/gist_id}","starred_url":"https://api.github.com/users/rrnewton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rrnewton/subscriptions","organizations_url":"https://api.github.com/users/rrnewton/orgs","repos_url":"https://api.github.com/users/rrnewton/repos","events_url":"https://api.github.com/users/rrnewton/events{/privacy}","received_events_url":"https://api.github.com/users/rrnewton/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/rrnewton/haskell-lockfree/labels/BUG","name":"BUG","color":"e10c02"},{"url":"https://api.github.com/repos/rrnewton/haskell-lockfree/labels/package%3Aatomic-primops","name":"package:atomic-primops","color":"fad8c7"}],"state":"open","locked":false,"assignee":{"login":"peter-fogg","id":1819831,"avatar_url":"https://avatars.githubusercontent.com/u/1819831?v=3","gravatar_id":"","url":"https://api.github.com/users/peter-fogg","html_url":"https://github.com/peter-fogg","followers_url":"https://api.github.com/users/peter-fogg/followers","following_url":"https://api.github.com/users/peter-fogg/following{/other_user}","gists_url":"https://api.github.com/users/peter-fogg/gists{/gist_id}","starred_url":"https://api.github.com/users/peter-fogg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/peter-fogg/subscriptions","organizations_url":"https://api.github.com/users/peter-fogg/orgs","repos_url":"https://api.github.com/users/peter-fogg/repos","events_url":"https://api.github.com/users/peter-fogg/events{/privacy}","received_events_url":"https://api.github.com/users/peter-fogg/received_events","type":"User","site_admin":false},"milestone":{"url":"https://api.github.com/repos/rrnewton/haskell-lockfree/milestones/1","labels_url":"https://api.github.com/repos/rrnewton/haskell-lockfree/milestones/1/labels","id":920456,"number":1,"title":"ghc-7.10","description":null,"creator":{"login":"rrnewton","id":274582,"avatar_url":"https://avatars.githubusercontent.com/u/274582?v=3","gravatar_id":"","url":"https://api.github.com/users/rrnewton","html_url":"https://github.com/rrnewton","followers_url":"https://api.github.com/users/rrnewton/followers","following_url":"https://api.github.com/users/rrnewton/following{/other_user}","gists_url":"https://api.github.com/users/rrnewton/gists{/gist_id}","starred_url":"https://api.github.com/users/rrnewton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rrnewton/subscriptions","organizations_url":"https://api.github.com/users/rrnewton/orgs","repos_url":"https://api.github.com/users/rrnewton/repos","events_url":"https://api.github.com/users/rrnewton/events{/privacy}","received_events_url":"https://api.github.com/users/rrnewton/received_events","type":"User","site_admin":false},"open_issues":3,"closed_issues":1,"state":"open","created_at":"2015-01-03T16:01:46Z","updated_at":"2015-01-03T17:14:42Z","due_on":null,"closed_at":null},"comments":4,"created_at":"2015-01-02T20:15:35Z","updated_at":"2015-01-03T17:39:14Z","closed_at":null,"body":"Ok, it looks like maybe this is an intentional change in behavior for `fetchAddIntArray#`, @tibbe?\r\n\r\nFollowing some precedent or existing code (I forgot which!), I'd made the original version return the AFTER value not the BEFORE value.  Thus the code for incrCounter was:\r\n\r\n```Haskell\r\n{-# INLINE incrCounter #-}\r\n-- | Increment the counter by a given amount.  Returns the value AFTER the increment\r\n--   (in contrast with the behavior of the underlying instruction on architectures\r\n--   like x86.)\r\n--\r\n--   Note that UNLIKE with boxed implementations of counters, where increment is\r\n--   based on CAS, this increment is /O(1)/.  Fetch-and-add does not require a retry\r\n--   loop like CAS.\r\nincrCounter :: Int -> AtomicCounter -> IO Int\r\nincrCounter (I# incr#) (AtomicCounter mba#) = IO $ \\ s1# ->\r\n  let (# s2#, res #) = fetchAddIntArray# mba# 0# incr# s1# in\r\n  (# s2#, (I# res) #)\r\n```\r\n\r\nIt looks like the 7.10+ version of fetchAddIntArray# returns the BEFORE value.  GCC provides both, `fetch_and_add` and `add_and_fetch`.  It's true that the primop we export matches the *name* of the former more closely, and it thus is fine for it to have that behavior.\r\n\r\nUh... it looks like we need another ifdef, eh, to support this change?  Oh well, that's the goal of the atomic-primops package.  To abstract over these varying levels of support in different GHC versions.\r\n\r\nBut I do think we should go ahead and bump the major version and change the interface for this Counter lib to return the BEFORE value (and support *that* behavior across the last 3 GHC versions), what do you think?\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/rrnewton/haskell-lockfree/issues/comments/68602731","html_url":"https://github.com/rrnewton/haskell-lockfree/issues/41#issuecomment-68602731","issue_url":"https://api.github.com/repos/rrnewton/haskell-lockfree/issues/41","id":68602731,"user":{"login":"jberryman","id":210815,"avatar_url":"https://avatars.githubusercontent.com/u/210815?v=3","gravatar_id":"","url":"https://api.github.com/users/jberryman","html_url":"https://github.com/jberryman","followers_url":"https://api.github.com/users/jberryman/followers","following_url":"https://api.github.com/users/jberryman/following{/other_user}","gists_url":"https://api.github.com/users/jberryman/gists{/gist_id}","starred_url":"https://api.github.com/users/jberryman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jberryman/subscriptions","organizations_url":"https://api.github.com/users/jberryman/orgs","repos_url":"https://api.github.com/users/jberryman/repos","events_url":"https://api.github.com/users/jberryman/events{/privacy}","received_events_url":"https://api.github.com/users/jberryman/received_events","type":"User","site_admin":false},"created_at":"2015-01-03T17:39:14Z","updated_at":"2015-01-03T17:39:14Z","body":"> Can we think of any other names that would be equally good that you can use going forward (with this new behavior), and just deprecate incrCounter and fetchAddByteArray (keeping the old behavior there)?\r\n\r\nI propose the following; let me know your thoughts:\r\n\r\n1.  Just using the names from `GHC.Prim` for the `fetch*` functions. So `fetchAddByteArrayInt` would be deprecated, with CPP conditionals to keep the behavior the same through 7.10. While a new wrapper named `fetchAddIntArray` would take its place with the new behavior (again conditionally bringing this NEW behavior to GHC <7.10). Likewise for the new functions (#43). This would avoid breakage, and I think make more sense (e.g. that's what the `primitive` package seems to do).\r\n\r\n2. Do one of the following:\r\n\r\n   - Remove all the `Data.Atomic.Counter*` modules; it's trivial to implement the functionality using `primitive`, the package is called `atomic-primops`, it's not clear to me why those alternate submodules are there, means less code to maintain, etc.\r\n\r\n    - Rewrite `incrCounter` with CPP conditionals to keep the *old* behavior going forward.\r\n\r\n3. Deprecate the new `0.6.1.1`, or adjust the `base` constraint back to `<=4.8`"}}