#9281 closed task (fixed)
Rewrite `integer-gmp` to use only non-allocating GMP functions
Reported by: | hvr | Owned by: | hvr |
---|---|---|---|
Priority: | normal | Milestone: | 7.10.1 |
Component: | Core Libraries | Version: | |
Keywords: | integer-gmp | Cc: | simonmar, core-libraries-committee@… |
Operating System: | Unknown/Multiple | Architecture: | Unknown/Multiple |
Type of failure: | None/Unknown | Test Case: | |
Blocked By: | Blocking: | ||
Related Tickets: | #8647 #9818 | Differential Rev(s): | Phab:D82 |
Wiki Page: |
Description (last modified by )
After some promising results with a proof-of-concept implementation, I'm optimistic we can rewrite integer-gmp
to use only non-allocating GMP lib functions without suffering from serious regressions.
If successful, this would
- allow to avoid the custom GMP allocator hack, and thus
- avoid issues when linking against other C libraries using GMP,
- simplify code, as we would perform all heap allocations in Haskell code (and never inside Cmm/C code as its done now),
- and finally maybe even remove a few more superfluous temporary heap allocations.
see also wiki:Design/IntegerGmp2
Change History (33)
comment:1 Changed 5 years ago by
comment:2 Changed 5 years ago by
Eliminating the need for the GMP custom allocator would be a _huge_ win for interoperability, and would obviate the need for code I've spent months writing.
If you need anything from me just ask.
comment:3 Changed 5 years ago by
Sounds fantastic to me. Do please document the design so that others do not later accidentally slip back into calling allocating functions!
Simon
comment:4 Changed 5 years ago by
Differential Rev(s): | → Phab:D82 |
---|---|
Related Tickets: | → #8647 |
Status: | new → patch |
Here's a first draft version. The number of library calls going into GMP (via ltrace -c
, which btw is a wonderful tool for that) is essentially the same (in some cases slightly lower, as the new code is a bit more clever in some ways). However, I'm seeing improvements as well as regressions in the Nofib results (see Phab:P6). I still need to figure out what the reason is for the rather visible allocation increase in primetest
(and maybe kahan
and bernouilli
).
It could be this different approach undoes part of #8647, as we can't so easily optimistically allocate temporary single-limbs on the stack (on the bright side, we've got almost no Cmm code anymore...)
Btw, while implementing the code, I would have wanted to write
foreign import ccall unsafe "integer_gmp_mpn_tdiv_q" c_mpn_tdiv_q :: MutableByteArray# s -> ByteArray# -> GmpSize# -> ByteArray# -> GmpSize# -> State# s -> State# s foreign import ccall unsafe "gmp.h __gmpn_divrem_1" c_mpn_divrem_1 :: MutableByteArray# s -> GmpSize# -> ByteArray# -> GmpSize# -> GmpLimb# -> State# s -> (# s, Word# #)
However, GHC insisted on having me to write ... -> IO ()
and ... -> IO Word
respectively, thus forcing a boxed/lifted Word
even though {-# LANGUAGE UnliftedFFITypes #-}
was in effect. Any chance to have unlifted types allowed for IO
-like FFI ccalls to reduce the overhead?
comment:5 follow-up: 6 Changed 5 years ago by
Terrific. It would be good to articulate the goal more explicitly. Is it primarily to improve interop in some way? How does it improve interop? Or is it to do with performance -- unsafe foreign calls are vastly more efficient than safe ones?
Why do integer_gmp_mpn_tdiv_q
etc need to be IO-ish at all? Can't they be pure functions?
Simon
comment:6 Changed 5 years ago by
Cc: | simonmar added |
---|
Replying to simonpj:
Terrific. It would be good to articulate the goal more explicitly. Is it primarily to improve interop in some way? How does it improve interop? Or is it to do with performance -- unsafe foreign calls are vastly more efficient than safe ones?
Are you referring to allowing unlifted types as return values in non-pure calls?
Why do
integer_gmp_mpn_tdiv_q
etc need to be IO-ish at all? Can't they be pure functions?
because those operations write into MutableByteArray#
comment:7 Changed 5 years ago by
What happens if you try
foreign import ccall unsafe "integer_gmp_mpn_tdiv_q" c_mpn_tdiv_q :: MutableByteArray# s -> ByteArray# -> GmpSize# -> ByteArray# -> GmpSize# -> State# s -> State# s
with -XUnliftedFFITypes
? Answer
Unacceptable argument type in foreign declaration: State# s
But why is State# s
unacceptable? I can think of no good reason.
So I think that it might be as simple as
- Adding
State#
to the list of acceptable FFI types (with-XUnliftedFFITypes
. - Making sure that, since it's a zero-width value, we don't actually pass it to the C function.
The latter step is in the code generator itself; these zero-width values do (and must) survive into Cmm
.
If you'd like to tackle this I'm sure Simon and I would help with any bits you don't grok.
I think this would be MUCH simpler than trying to allow types like IO Int#
, which has global implications that I don't know how to deal with.
Simon
comment:8 Changed 5 years ago by
fwiw, if you use prim
-FFI imports instead of ccall
, ... -> State# s -> State# s
as well as ... -> State# s -> (# State# s, Word# #)
already works. Is this maybe just a matter of replicating what's already done for prim
-style FFI/Cmm calls?
comment:9 Changed 5 years ago by
foreign import prim
has a different calling convention from foreign import ccall
(obviously) and they look different at the Cmm
level. The code generation for these two is quite different - the former is a StgPrimCallOp
while the latter is a StgFCallOp
.
But I think all the machinery to do this already exists, as Simon said. When we foreign-import something with an IO type there's a wrapper around the primitive foreign call to add the IO, but the primitive call itself will have a type of the form State# s -> (# State# s, a #)
, so we can already codegen these properly. All we need to do is make sure that there is no wrapper, which might already happen automatically if the typechecker is changed to allow State#
arguments and State#
or (# State# s, a #)
results with UnliftedFFITypes
.
comment:10 Changed 5 years ago by
Fyi, I've created a separate ticket #9352 to further discuss the State# s
/FFI story
comment:13 Changed 4 years ago by
Component: | libraries (other) → Core Libraries |
---|
Moving over to new owning component 'Core Libraries'.
comment:14 Changed 4 years ago by
Cc: | core-libraries-committee@… added |
---|---|
Description: | modified (diff) |
started a wiki-page over at wiki:Design/IntegerGmp2
comment:30 Changed 4 years ago by
Resolution: | → fixed |
---|---|
Status: | patch → closed |
Let's consider this accomplished for now.
comment:31 Changed 4 years ago by
Related Tickets: | #8647 → #8647 #9818 |
---|
As a brain-dump, here's the new representation I'm working on (which happens to make it trivial to provide a natural number type
Nat
):