Make GHC (the library) flexible in the choice of integer library
ClosedPublic

Authored by nomeata on Aug 20 2018, 4:47 PM.

Details

Summary

We have more and more users of GHC as a library, for example the Haskell-to-WebAssembly-compiler https://github.com/tweag/asterius. These need to make different decisions about various aspects of code generation than the host compiler, and ideally GHC-the-library allows them to set the DynFlags as needed.

This patch adds a new DynFlag that configures which integer library to use. This flag is initialized by cIntegerLibraryType (as before), and is only used in CorePrep to decide whether to use S# or not.

The other code paths that were varying based on cIntegerLibraryType are no now longer varying: The trick is to use integer-wired-in as the -this-unit-id when compiling either integer-gmp or integer-simple.

Test Plan

Validate is happy.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
nomeata created this revision.Aug 20 2018, 4:47 PM
nomeata updated this revision to Diff 17705.Aug 20 2018, 4:53 PM
  • Introduce integerLibrary :: DynFlags -> IntegerLibrary
nomeata updated this revision to Diff 17706.Aug 20 2018, 5:45 PM
  • Introduce integerLibrary :: DynFlags -> IntegerLibrary
nomeata updated this revision to Diff 17707.Aug 20 2018, 6:35 PM
  • Update findWiredInPackages to handle new integer unit name
nomeata updated this revision to Diff 17711.Aug 20 2018, 7:38 PM
  • Unused import
nomeata retitled this revision from [wip] Make GHC (the library) flexible in the choice of integer library to Make GHC (the library) flexible in the choice of integer library.Aug 20 2018, 11:05 PM
nomeata edited the summary of this revision. (Show Details)
nomeata edited the test plan for this revision. (Show Details)
nomeata updated the Trac tickets for this revision.

I'm impressed that you worked out all the moving parts so well.

Now that you understand them, could you write a Note [Selecting which integer library to use], and refer to it from all these subtle places?

bgamari requested changes to this revision.Aug 21 2018, 10:41 AM

Yes, a note would be quite handy.

libraries/integer-gmp/integer-gmp.cabal
63–66

Neat!

This revision now requires changes to proceed.Aug 21 2018, 10:41 AM
nomeata updated this revision to Diff 17716.Aug 21 2018, 11:42 AM
  • Add a Note and and use integer-wired-in for the unit name
nomeata updated this revision to Diff 17717.Aug 21 2018, 11:55 AM
  • One more note reference
adamse added a subscriber: adamse.Aug 26 2018, 10:39 AM

I think the sumarry has diverged from the code: "The trick is to use integer as the -this-unit-id" should be "use integer-wired-in".

nomeata edited the summary of this revision. (Show Details)Aug 26 2018, 9:27 PM

Dev of asterius here. This patch already works in asterius, allowing using a ghc built with integer-gmp/tables-next-to-code to compile Haskell to WebAssembly with integer-simple/no-tables-next-to-code. Can we proceed with merging this differential before it gets bit-rotten? Anything I can offer here? Thank you.

simonpj added inline comments.Sep 7 2018, 2:36 AM
compiler/main/Packages.hs
1061

What's going on here?

Indeed a note to explain what a "Wired-in package" is and why they are special, would be helpful.

compiler/prelude/PrelNames.hs
115

Great Note. But as you'll see I think it could be clearer

131

Is the following true?

So the effect is that when compiling (say) integer-gmp, GHC thinks the package is called "integer-wired-in", and it'll be installed as a package "integer-wired-in". It's rather as if we began by copying integer-gmp/* (or integer-simple/*) into integer-wired-in/*, and then compiled integer-wired-in package normally.

Eg. is the package really installed as "integer-wired-in"? If not, if it's installed as integer-gmp, it'd be the only package whose .hi files disagreed with its installation name.

If true, can we say so? And wouldn't that be a simpler implementation technique too???!

142

I did not understand this bit

nomeata marked an inline comment as done.Sep 7 2018, 4:07 AM

More note massaging.

compiler/main/Packages.hs
1061

I did not fundamentally change anything here… but I guess this is the GHC “improve existing documentation tax” on contributions :-)

The note for that is in Module, with the non-standard note header -- $wired_in_packages.

Will add a reference.

compiler/prelude/PrelNames.hs
142

Added a link to the note that explains the this (general, not integer-specific) functionality; hope that helps.

nomeata updated this revision to Diff 17937.Sep 7 2018, 4:07 AM
nomeata marked an inline comment as done.
  • More links between notes, in particular Note [Wired-in packages]
simonpj added inline comments.Sep 7 2018, 8:38 AM
compiler/basicTypes/Module.hs
1045

Helpful, thank you

compiler/prelude/PrelNames.hs
131

This part still not yet addressed?

142

Better!

nomeata added inline comments.Sep 7 2018, 8:52 AM
compiler/prelude/PrelNames.hs
131

The effect would not be the same.

If you run ghc-pkg describe base, you will see that the id is base-4.11.0.0, and *not* the internal unit id base.

Similarly, with this patch, if you run ghc-pkg describe integer-gmp, you will see an id integer-gmp-1.2.3.4, not integer-wired-in.

If you just copy the packages, then this would be externally visible – and possibly confuse our users and cabal.

Hence, I assume, the existing scheme was chosen, where only internally these names are mangled.

simonpj added inline comments.Sep 7 2018, 9:17 AM
compiler/prelude/PrelNames.hs
131

OK so we can say that the package is still installed as integer-gmp-1.2.3.4, but all the symbols in it, and its .hi files, look like integer-wired-in:M.blah. If that is right. I'm not arguing for a change -- just wanting the Note to be more precise. Thanks!

nomeata updated this revision to Diff 17942.Sep 7 2018, 9:52 AM

More note refinement. Most of the information seems to be in Note [Wired-in
packages] already, but I rephrased slightly.

nomeata added inline comments.Sep 7 2018, 9:53 AM
compiler/prelude/PrelNames.hs
131

Refined the existing note (hopefully) to that effect

Ben, this PR is still officially blocked on you. Do you want to check it again? I hope I did enough Note refinement for now :-)

bgamari accepted this revision.Oct 2 2018, 11:10 AM

Ahh, I missed that you had revised it. Thanks for the ping!

This looks quite reasonable; however, I left a question inline.

compiler/basicTypes/Module.hs
1061

What happens if the user installs integer-simple and attempts to set -package integer-simple, for instance, in GHCi.

This revision is now accepted and ready to land.Oct 2 2018, 11:10 AM
nomeata updated this revision to Diff 18179.Oct 2 2018, 11:31 AM
  • Merge branch 'master' of git://git.haskell.org/ghc into wip/T13477 Will merge into master when Phab is happy.
nomeata added inline comments.Oct 2 2018, 11:31 AM
compiler/basicTypes/Module.hs
1061
$ cabal install -w ./inplace/bin/ghc-stage2  integer-simple
Resolving dependencies...
cabal: Could not resolve dependencies:
[__0] next goal: integer-simple (user goal)
[__0] rejecting: integer-simple-0.1.1.1 (only already installed instances can
be used)
[__0] fail (backjumping, conflict set: integer-simple)
After searching the rest of the dependency tree exhaustively, these were the
goals I've had most trouble fulfilling: integer-simple

looks like they can’t. This is not new :-)

This revision was automatically updated to reflect the committed changes.