[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
#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]:
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.
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 <duncan@well-typed.com> Date: Fri Sep 13 09:19:24 2013 +0100 New primops for byte range copies ByteArray# <-> Addr# We have primops for copying ranges of bytes between ByteArray#s: * ByteArray# -> MutableByteArray# * MutableByteArray# -> MutableByteArray# This extends it with three further cases: * Addr# -> MutableByteArray# * ByteArray# -> Addr# * MutableByteArray# -> Addr# One use case for these is copying between ForeignPtr-based representations and in-heap arrays (like Text, UArray etc). The implementation is essentially the same as for the existing primops, and shares the memcpy stuff in the code generators. Defficiencies / future directions: none of these primops (existing or the new ones) let one take advantage of knowing that ByteArray#s are word-aligned in memory. Though it is unclear that any of the code generators would make use of this information unless the size to copy is also known at compile time. Signed-off-by: Austin Seipp <austin@well-typed.com> }}} Duncan, I also had to fix the actual primops in bb532682aa47c30dfd49039c5ab282352d38dac0, since they claimed to work over `ST RealWorld`, but instead should work over `ST s` {{{ commit bb532682aa47c30dfd49039c5ab282352d38dac0 Author: Austin Seipp <austin@well-typed.com> Date: Sun Sep 15 15:15:17 2013 -0500 Fix the type signatures of new copy primops. They claimed to work over 'ST RealWorld', when instead they should be parameterized in the state type. This fixes the cgrun070. Signed-off-by: Austin Seipp <austin@well-typed.com> }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/8296#comment:6> 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): 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