Re: Proposal: Move primitive-Data.Primitive.Addr API into base

https://phabricator.haskell.org/D5268 On Fri, Oct 26, 2018 at 1:08 PM Carter Schonwald wrote:

On the flip side we could just add data Addr ... to foreign .ptr or a child / sibling module like System.Mem.Address and defer the associated cleanup

I kinda like the idea of System.Mem.Address,

i put a strawman change set on phab https://phabricator.haskell.org/D5268

On Fri, Oct 26, 2018 at 12:23 PM Carter Schonwald < carter.schonwald@gmail.com> wrote:

...

granted most of the Addr# sites dont matter

a related issue is what operations WOULD be added for Addr?

maybe just memcopy friends plus address arithmetic, plus a mapper from/to Ptr a?

does it simply become a byte size step sibling to Ptr a that put in Foreign.Ptr?

On Fri, Oct 26, 2018 at 12:18 PM Carter Schonwald < carter.schonwald@gmail.com> wrote:

...

Before we bike shed these details, let’s evaluate / document the pieces of base that should also be moved to using Addr. Though that module where

1) What exported types / data definitions / functions will have changes ?

2) what internal apis need to change?

as a lower bound estimate of work surface area, i grepped for "Ptr ()" and "Addr#" in a fresh recursive clone of ghc at commit 503514b94f8dc7bd9eab5392206649aee45f140b, limiting my search to .hs and .hsc files

THERES A LOT of uses of Ptr (), and its siblings StablePtr () and RemotePtr () , The latter two dont have a simple Addr# underneath,

heres the list of files that match that search rg 'Ptr \(\)' --type haskell -c

ghc/GHCi/Leak.hs:1 utils/hsc2hs/Main.hs:1 utils/runghc/Main.hs:1 compiler/ghci/GHCi.hs:4 compiler/ghci/ByteCodeLink.hs:3 compiler/ghci/ByteCodeGen.hs:4 compiler/ghci/ByteCodeTypes.hs:1 compiler/ghci/ByteCodeAsm.hs:2 compiler/main/HscTypes.hs:1 libraries/ghci/GHCi/ObjLink.hs:3 libraries/ghci/GHCi/StaticPtrTable.hs:1 libraries/ghci/GHCi/ResolvedBCO.hs:1 libraries/ghci/GHCi/RemoteTypes.hs:2 libraries/ghci/GHCi/Run.hs:2 libraries/ghci/GHCi/Message.hs:5 libraries/base/GHC/Stable.hs:2 libraries/base/GHC/StaticPtr.hs:1 libraries/base/GHC/Generics.hs:2 libraries/base/GHC/TopHandler.hs:1 libraries/base/GHC/Windows.hs:1 libraries/base/tests/dynamic002.hs:2 utils/haddock/haddock-api/src/Haddock.hs:1 libraries/array/Data/Array/IO.hs:2 libraries/base/Foreign/Marshal/Pool.hs:1 libraries/bytestring/Data/ByteString/Builder/Extra.hs:1 libraries/base/GHC/IO/Handle/Text.hs:1 testsuite/tests/rts/T7160.hs:1 testsuite/tests/rts/overflow3.hs:1 testsuite/tests/rts/overflow2.hs:1 testsuite/tests/rts/T5250.hs:1 testsuite/tests/rts/T10296b.hs:2 testsuite/tests/th/T3319.hs:1 testsuite/tests/th/TH_foreignInterruptible.hs:1 testsuite/tests/th/TH_foreignCallingConventions.hs:1 libraries/Cabal/cabal-install/Distribution/Client/Win32SelfUpgrade.hs:1 libraries/Cabal/Cabal/Distribution/Simple/InstallDirs.hs:2 testsuite/tests/llvm/should_compile/T5486.hs:2 testsuite/tests/codeGen/should_run/cgrun070.hs:5 testsuite/tests/codeGen/should_run/cgrun026.hs:3 testsuite/tests/primops/should_run/T4442.hs:1 testsuite/tests/ffi/should_fail/T7243.hs:1 testsuite/tests/ffi/should_fail/T3066.hs:1 testsuite/tests/ffi/should_compile/cc009.hs:1 testsuite/tests/ffi/should_run/ffi021.hs:1 testsuite/tests/typecheck/should_compile/T7408.hs:1

libraries/Cabal/cabal-install/Distribution/Client/Compat/ExecutablePath.hs:1 libraries/Cabal/Cabal/Distribution/Simple/Build/PathsModule.hs:1 libraries/time/lib/Data/Time/Clock/Internal/CTimeval.hs:1 testsuite/tests/rename/should_compile/T3103/Foreign/Ptr.hs:1

likewise the locations where Addr# comes up are as follows (some of these perhaps should be an Addr? )

$ rg 'Addr#' --type haskell -c utils/genprimopcode/Main.hs:2 ghc/GHCi/Leak.hs:2 compiler/cmm/CmmMachOp.hs:1 compiler/cmm/CmmUtils.hs:1 compiler/specialise/Rules.hs:2 compiler/basicTypes/MkId.hs:2 compiler/basicTypes/Literal.hs:6 compiler/basicTypes/Unique.hs:2 compiler/utils/StringBuffer.hs:1 compiler/utils/FastString.hs:4 compiler/utils/Encoding.hs:9 compiler/codeGen/StgCmmPrim.hs:3 compiler/coreSyn/MkCore.hs:5 compiler/coreSyn/CoreLint.hs:2 compiler/coreSyn/CoreSyn.hs:4 compiler/coreSyn/CorePrep.hs:1 compiler/stranal/WwLib.hs:1 compiler/simplCore/CSE.hs:1 compiler/simplCore/Simplify.hs:2 compiler/deSugar/DsExpr.hs:1 compiler/typecheck/TcGenDeriv.hs:5 compiler/prelude/ForeignCall.hs:1 compiler/prelude/PrelNames.hs:1 compiler/prelude/TysPrim.hs:1 compiler/ghci/ByteCodeGen.hs:8 libraries/bytestring/tests/revcomp.hs:1 libraries/bytestring/Data/ByteString.hs:5 libraries/ghc-heap/tests/heap_all.hs:1 libraries/ghc-prim/GHC/CString.hs:17 libraries/ghc-prim/GHC/Classes.hs:1 libraries/ghc-prim/GHC/Types.hs:3 libraries/ghci/GHCi/BinaryArray.hs:1 libraries/ghci/GHCi/CreateBCO.hs:5 libraries/base/codepages/MakeTable.hs:1 libraries/base/GHC/Ptr.hs:8 libraries/base/GHC/Generics.hs:5 libraries/base/GHC/Storable.hs:32 libraries/base/GHC/ForeignPtr.hs:6 libraries/base/Foreign/Ptr.hs:2 libraries/bytestring/Data/ByteString/Unsafe.hs:6 libraries/bytestring/Data/ByteString/Internal.hs:7 libraries/ghc-heap/GHC/Exts/Heap.hs:1 libraries/text/Data/Text/Show.hs:2 libraries/text/Data/Text/Lazy.hs:2 libraries/containers/Data/IntSet/Internal.hs:4 libraries/ghc-compact/GHC/Compact/Serialized.hs:11 libraries/base/GHC/IO/Exception.hs:1 libraries/base/Data/Typeable/Internal.hs:3 libraries/base/Control/Exception/Base.hs:1 utils/haddock/html-test/src/Ticket112.hs:1 testsuite/tests/rts/T8242.hs:19 testsuite/tests/ghci.debugger/HappyTest.hs:3 testsuite/tests/arityanal/Main.hs:3 testsuite/tests/th/TH_StringPrimL.hs:2 testsuite/tests/th/T1830_3a.hs:2 libraries/template-haskell/Language/Haskell/TH/Syntax.hs:1 libraries/bytestring/Data/ByteString/Short/Internal.hs:16 libraries/integer-gmp/src/GHC/Integer/Type.hs:11 libraries/base/GHC/IO/Encoding/CodePage.hs:3 libraries/Cabal/Cabal/Distribution/Parsec/Lexer.hs:9 testsuite/tests/generics/GEq/GEq1A.hs:1 testsuite/tests/generics/GEq/GEq1.hs:2 testsuite/tests/codeGen/should_compile/cg008.hs:1 testsuite/tests/codeGen/should_run/cgrun070.hs:4 testsuite/tests/codeGen/should_run/T9577.hs:1 testsuite/tests/codeGen/should_run/cgrun066.hs:1 testsuite/tests/codeGen/should_run/cgrun026.hs:2 testsuite/tests/codeGen/should_run/T2080.hs:2 testsuite/tests/codeGen/should_run/T9577_A.hs:1 testsuite/tests/primops/should_run/T4442.hs:4 testsuite/tests/simplCore/should_compile/T13367.hs:1 testsuite/tests/simplCore/should_compile/T13155.hs:2 testsuite/tests/simplCore/should_compile/T14779a.hs:1 testsuite/tests/simplCore/should_compile/T14779b.hs:2 testsuite/tests/simplCore/should_compile/spec001.hs:1 testsuite/tests/lib/integer/integerGmpInternals.hs:9 testsuite/tests/perf/compiler/T14683.hs:3 testsuite/tests/ghci/should_run/T2589.hs:1 libraries/integer-gmp/src/GHC/Integer/GMP/Internals.hs:7 libraries/text/Data/Text/Internal/Fusion/Common.hs:3 libraries/base/GHC/IO/Encoding/CodePage/Table.hs:1

On Fri, Oct 26, 2018 at 11:01 AM Daniel Cartwright < chessai1996@gmail.com> wrote:

...

Also, where in base should Addr go?

On Fri, Oct 26, 2018 at 10:15 AM Daniel Cartwright < chessai1996@gmail.com> wrote:

...

I would, yeah. Should the next version of `primitive` just re-export `Addr` from base in `Data.Primitive.Addr`?

On Fri, Oct 26, 2018 at 10:14 AM Carter Schonwald < carter.schonwald@gmail.com> wrote:

...

Daniel or David, would either of you be interested/willing to do the legwork on this patch wise (subject to sanity checking whats the impact surface area?)

On Fri, Oct 26, 2018 at 12:04 AM Carter Schonwald < carter.schonwald@gmail.com> wrote:

> Cool. What apis in base should move to use Addr from fake pointer ? > Cause just adding it in isolation seems lame! > > I guess I’m just trying to say “what would the impact on base, if > every fake Ptr was moved to be Addr?” Because it’s not something which > should be done by halves. > > 1) what apis would be changed? > > 2) what internal things would be changed albeit not Api visible > > 3). Address arithmetic for Addr is very different from addres > arithmetic on larger than word8 size storable values, is there anything > which would move representations where the change to byte indexed would > change the calculations ? > > > If we can layout what the impact / scope of changes needed for these > and other considerations, I guess I’d be all for it. Assuming there’s the > corresponding interest in executing changes. (I just have this worry in my > head that it might be one of those icebergs.. like the split base effort ) > > > > On Thu, Oct 25, 2018 at 11:33 PM David Feuer > wrote: > >> On Thu, Oct 25, 2018, 11:28 PM Carter Schonwald < >> carter.schonwald@gmail.com> wrote: >> >>> Pretending we’re talking about this with storable as our example >>> semantics : >>> >>> Ptr Void >>> Corresponds to a memory address you don’t want to read or write to. >>> >> >> This is not so easy to think about (there's not much utility in >> such a type). Perhaps Ptr Void is a reasonable type for pointers known to >> be null or outside the address space, but I'm not sure. >> >> >>> Ptr () corresponds to a memory location that’s pretty boring to >>> read / write to >>> >> >> Yes! In other words, it's *not* what people should be using for a >> pointer to an arbitrary non-Haskell address. That's precisely what Addr is >> for. >> >> >>> On Thu, Oct 25, 2018 at 11:22 PM Carter Schonwald < >>> carter.schonwald@gmail.com> wrote: >>> >>>> ... when is a valid pointer not going to point at byte >>>> addressable memory on memory architectures ghc can support or target? >>>> >>>> On Thu, Oct 25, 2018 at 8:27 PM Daniel Cartwright < >>>> chessai1996@gmail.com> wrote: >>>> >>>>> Now, one could argue that `Ptr ()` isn't a lie, it sort of reads >>>>> like C's void pointer. But surely something like `Ptr Word8` is a lie, when >>>>> it is not actually a Ptr to Word8 values. >>>>> >>>>> On Thu, Oct 25, 2018 at 8:11 PM Daniel Cartwright < >>>>> chessai1996@gmail.com> wrote: >>>>> >>>>>> yes, only the type and its instances should be moved as far as >>>>>> i'm aware. >>>>>> >>>>>> Also, it's more than just base. >>>>>> >>>>>> this Ptr is a lie: >>>>>> https://hackage.haskell.org/package/ghc-compact-0.1.0.0/docs/GHC-Compact-Ser... >>>>>> these Ptrs are lies: >>>>>> https://hackage.haskell.org/package/base-4.12.0.0/docs/GHC-IO-Handle.html >>>>>> in GHC.Stats, the foreign import "getRTSStats" has `Ptr () -> >>>>>> IO ()`, this Ptr () is also a lie >>>>>> >>>>>> These are just off the top of my head, there are more >>>>>> >>>>>> >>>>>> On Thu, Oct 25, 2018 at 6:46 PM Carter Schonwald < >>>>>> carter.schonwald@gmail.com> wrote: >>>>>> >>>>>>> hrmm, what are the pieces of base that are using Ptr when they >>>>>>> really should be using Addr? This would help me understand what would be >>>>>>> made better in base :) >>>>>>> >>>>>>> On Thu, Oct 25, 2018 at 6:19 PM David Feuer < >>>>>>> david.feuer@gmail.com> wrote: >>>>>>> >>>>>>>> We shouldn't really need to move anything into base except >>>>>>>> Addr and its base instances. >>>>>>>> >>>>>>>> On Oct 25, 2018 5:59 PM, "Carter Schonwald" < >>>>>>>> carter.schonwald@gmail.com> wrote: >>>>>>>> >>>>>>>> Indeed. The monad transformer instances for primmonad need >>>>>>>> to live in primmonad OR transformers to avoid orphans. >>>>>>>> >>>>>>>> Either way, unless transformers moves into base (unlikely), >>>>>>>> no way anything using prim monad will. >>>>>>>> >>>>>>>> On Thu, Oct 25, 2018 at 3:34 PM Andrew Martin < >>>>>>>> andrew.thaddeus@gmail.com> wrote: >>>>>>>> >>>>>>>>> I like the idea of moving the type Addr into base. But we >>>>>>>>> cannot move the entire module since it has functions that talk about >>>>>>>>> PrimMonad, and we definitely don't want to move that into base. >>>>>>>>> >>>>>>>>> On Thu, Oct 25, 2018 at 11:25 AM Daniel Cartwright < >>>>>>>>> chessai1996@gmail.com> wrote: >>>>>>>>> >>>>>>>>>> Motivation: There are a lot of places in base where 'Ptr a' >>>>>>>>>> is used in place of 'Addr', because in base there is no 'Addr', only >>>>>>>>>> 'Addr#'. The problem lies in the fact that many of these uses of 'Ptr a' >>>>>>>>>> are lying; the 'a' value is meaningless. Authors of functions therein have >>>>>>>>>> used things like 'Ptr ()', 'Ptr Word8', 'Ptr a', but these types do not >>>>>>>>>> mean what they say they mean - they're just Addr. There are probably other >>>>>>>>>> motivations for this that I can't think of off the top of my head right now. >>>>>>>>>> _______________________________________________ >>>>>>>>>> Libraries mailing list >>>>>>>>>> Libraries@haskell.org >>>>>>>>>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> -Andrew Thaddeus Martin >>>>>>>>> _______________________________________________ >>>>>>>>> Libraries mailing list >>>>>>>>> Libraries@haskell.org >>>>>>>>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries >>>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> Libraries mailing list >>>>>>>> Libraries@haskell.org >>>>>>>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries >>>>>>>> >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>> Libraries mailing list >>>>>>> Libraries@haskell.org >>>>>>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries >>>>>>> >>>>>>