[GHC] #8296: Patch: new primops for byte range copies ByteArray# <-> Addr#
#8296: Patch: new primops for byte range copies ByteArray# <-> Addr# ------------------------------------+------------------------------------- Reporter: duncan | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Keywords: | Operating System: Unknown/Multiple Architecture: Unknown/Multiple | Type of failure: None/Unknown Difficulty: Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | ------------------------------------+------------------------------------- Currently we have: * `copyByteArray#` for `ByteArray#` to `MutableByteArray#` * `copyMutableByteArray#` for `MutableByteArray#` to `MutableByteArray#` This patch adds primops for similar cases involving `Addr#`s, that is pointers to pinned or foreign memory. * `copyByteArrayToAddr#` for `ByteArray#` to `Addr#` * `copyMutableByteArrayToAddr#` for `MutableByteArray#` to `Addr#` * `copyAddrToByteArray#` for `Addr#` to `MutableByteArray#` These are not covered by the existing primops of course, and are useful because `ByteArray#`s are a bit special, sometimes being unpinned, and being aligned, and being so primitve/built-in. It's true we could use FFI import of memcpy using the GHC's FFI extension to turn `ByteArray#` into a pointer on the C side. However we don't do that for the existing primops and we're following the same pattern here. (Good reasons for them all to be primops: abstracts the backend/platform better, can take advantage of known allignment and size info). In particular, these primops would be useful in the impl of low level libs like bytestring, text, array, binary etc. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8296 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8296: Patch: new primops for byte range copies ByteArray# <-> Addr# -------------------------------------+------------------------------------ Reporter: duncan | Owner: Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by duncan): BTW, while I've compiled ghc with these and run the individual test, I've not done a full validate run. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8296#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8296: Patch: new primops for byte range copies ByteArray# <-> Addr# -------------------------------------+------------------------------------ Reporter: duncan | Owner: Type: feature request | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Changes (by duncan): * status: new => patch -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8296#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8296: Patch: new primops for byte range copies ByteArray# <-> Addr# -------------------------------------+------------------------------------ Reporter: duncan | Owner: Type: feature request | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by rwbarton): It looks like the comments in `doCopyByteArrayToAddrOp` and `doCopyAddrToByteArrayOp` were copied from `doCopyByteArrayOp`. Actually, the reason it's okay to assume the memory ranges aren't overlapping in the former two functions is that that is a precondition of the primop. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8296#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8296: Patch: new primops for byte range copies ByteArray# <-> Addr# -------------------------------------+------------------------------------ Reporter: duncan | Owner: Type: feature request | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by thoughtpolice): This patch looks quite straightforward to me. If nobody has any other objections, I'll put it on my patch queue and merge it later tonight. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8296#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
It looks like the comments in `doCopyByteArrayToAddrOp` and `doCopyAddrToByteArrayOp` were copied from `doCopyByteArrayOp`. Actually,
#8296: Patch: new primops for byte range copies ByteArray# <-> Addr# -------------------------------------+------------------------------------ Reporter: duncan | Owner: Type: feature request | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by duncan): Replying to [comment:3 rwbarton]: the reason it's okay to assume the memory ranges aren't overlapping in the former two functions is that that is a precondition of the primop. Fair point. Updated those two comments. Replying to [comment:4 thoughtpolice]:
This patch looks quite straightforward to me. If nobody has any other objections, I'll put it on my patch queue and merge it later tonight.
Ta! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8296#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8296: Patch: new primops for byte range copies ByteArray# <-> Addr#
-------------------------------------+------------------------------------
Reporter: duncan | Owner:
Type: feature request | Status: closed
Priority: normal | Milestone:
Component: Compiler | Version: 7.6.3
Resolution: fixed | Keywords:
Operating System: Unknown/Multiple | Architecture: Unknown/Multiple
Type of failure: None/Unknown | Difficulty: Unknown
Test Case: | Blocked By:
Blocking: | Related Tickets:
-------------------------------------+------------------------------------
Changes (by thoughtpolice):
* status: patch => closed
* resolution: => fixed
Comment:
Merged.
{{{
commit f11289f65e77b9e3178b08f5ca4472762c77c42e
Author: Duncan Coutts
#8296: Patch: new primops for byte range copies ByteArray# <-> Addr# -------------------------------------+------------------------------------ Reporter: duncan | Owner: Type: feature request | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by duncan): Oh sorry about that. I see that I changed the primops to use `State# RealWorld` after writing the tests and then didn't update the tests. So actually I think using `RealWorld` here is probably right, because these ops work with `Addr#`s, both reading and writing from them, and so this does not really make sense in a pure ST context. Admititly, the pattern in the existing primops is not totally clear about this, things like `touch#` use `State# RealWorld`, but then all the primops like `readIntOffAddr#` use `State# s`. So not totally clear cut either way. I'll leave the choice to you. If you want to switch back to using `State# RealWorld` as in my original patch, then I attach the corresponding fix to the tests. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8296#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8296: Patch: new primops for byte range copies ByteArray# <-> Addr# -------------------------------------+------------------------------------ Reporter: duncan | Owner: Type: feature request | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by duncan): On reflection, there's probably no harm in leaving the primops as `State# s`. It's vaguely plausible it could be useful in a context like repa/dph which use `Addr#`s pointing into pinned `ByteArray#`s. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8296#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC