[Git][ghc/ghc][wip/T24251a] 33 commits: Fix sharing of 'IfaceTyConInfo' during core to iface type translation

Simon Peyton Jones (@simonpj) gitlab at gitlab.haskell.org
Fri Mar 22 14:24:41 UTC 2024



Simon Peyton Jones pushed to branch wip/T24251a at Glasgow Haskell Compiler / GHC


Commits:
73be65ab by Fendor at 2024-03-19T01:42:53-04:00
Fix sharing of 'IfaceTyConInfo' during core to iface type translation

During heap analysis, we noticed that during generation of
'mi_extra_decls' we have lots of duplicates for the instances:

* `IfaceTyConInfo NotPromoted IfaceNormalTyCon`
* `IfaceTyConInfo IsPromoted IfaceNormalTyCon`

which should be shared instead of duplicated. This duplication increased
the number of live bytes by around 200MB while loading the agda codebase
into GHCi.

These instances are created during `CoreToIface` translation, in
particular `toIfaceTyCon`.

The generated core looks like:

    toIfaceTyCon
      = \ tc_sjJw ->
          case $wtoIfaceTyCon tc_sjJw of
          { (# ww_sjJz, ww1_sjNL, ww2_sjNM #) ->
          IfaceTyCon ww_sjJz (IfaceTyConInfo ww1_sjNL ww2_sjNM)
          }

whichs removes causes the sharing to work propery.

Adding explicit sharing, with NOINLINE annotations, changes the core to:

    toIfaceTyCon
      = \ tc_sjJq ->
          case $wtoIfaceTyCon tc_sjJq of { (# ww_sjNB, ww1_sjNC #) ->
          IfaceTyCon ww_sjNB ww1_sjNC
          }

which looks much more like sharing is happening.
We confirmed via ghc-debug that all duplications were eliminated and the
number of live bytes are noticeably reduced.

- - - - -
bd8209eb by Alan Zimmerman at 2024-03-19T01:43:28-04:00
EPA: Address more 9.10.1-alpha1 regressions from recent changes

Closes #24533
Hopefully for good this time

- - - - -
31bf85ee by Fendor at 2024-03-19T14:48:08-04:00
Escape multiple arguments in the settings file

Uses responseFile syntax.

The issue arises when GHC is installed on windows into a location that
has a space, for example the user name is 'Fake User'.
The $topdir will also contain a space, consequentially.
When we resolve the top dir in the string `-I$topdir/mingw/include`,
then `words` will turn this single argument into `-I/C/Users/Fake` and
`User/.../mingw/include` which trips up the flag argument parser of
various tools such as gcc or clang.
We avoid this by escaping the $topdir before replacing it in
`initSettngs`.
Additionally, we allow to escape spaces and quotation marks for
arguments in `settings` file.

Add regression test case to count the number of options after variable
expansion and argument escaping took place.
Additionally, we check that escaped spaces and double quotation marks are
correctly parsed.

- - - - -
f45f700e by Matthew Pickering at 2024-03-19T14:48:44-04:00
Read global package database from settings file

Before this patch, the global package database was always assumed to be
in libdir </> package.conf.d.

This causes issues in GHC's build system because there are sometimes
situations where the package database you need to use is not located in
the same place as the settings file.

* The stage1 compiler needs to use stage1 libraries, so we should set
  "Global Package DB" for the stage1 compiler to the stage1 package
  database.
* Stage 2 cross compilers need to use stage2 libraries, so likewise, we
  should set the package database path to `_build/stage2/lib/`

* The normal situation is where the stage2 compiler uses stage1
  libraries. Then everything lines up.

* When installing we have rearranged everything so that the settings
  file and package database line up properly, so then everything should
  continue to work as before. In this case we set the relative package
  db path to `package.conf.d`, so it resolves the same as before.

* ghc-pkg needs to be modified as well to look in the settings file fo
  the package database rather than assuming the global package database
  location relative to the lib folder.

* Cabal/cabal-install will work correctly because they query the global
  package database using `--print-global-package-db`.

A reasonable question is why not generate the "right" settings files in
the right places in GHC's build system. In order to do this you would
need to engineer wrappers for all executables to point to a specific
libdir. There are also situations where the same package db is used by
two different compilers with two different settings files (think stage2
cross compiler and stage3 compiler).

In short, this 10 line patch allows for some reasonable simplifications
in Hadrian at very little cost to anything else.

Fixes #24502

- - - - -
4c8f1794 by Matthew Pickering at 2024-03-19T14:48:44-04:00
hadrian: Remove stage1 testsuite wrappers logic

Now instead of producing wrappers which pass the global package database
argument to ghc and ghc-pkg, we write the location of the correct
package database into the settings file so you can just use the intree
compiler directly.

- - - - -
da0d8ba5 by Matthew Craven at 2024-03-19T14:49:20-04:00
Remove unused ghc-internal module "GHC.Internal.Constants"

- - - - -
b56d2761 by Matthew Craven at 2024-03-19T14:49:20-04:00
CorePrep: Rework lowering of BigNat# literals

Don't use bigNatFromWord#, because that's terrible:
 * We shouldn't have to traverse a linked list at run-time
   to build a BigNat# literal. That's just silly!
 * The static List object we have to create is much larger
   than the actual BigNat#'s contents, bloating code size.
 * We have to read the corresponding interface file,
   which causes un-tracked implicit dependencies. (#23942)

Instead, encode them into the appropriate platform-dependent
sequence of bytes, and generate code that copies these bytes
at run-time from an Addr# literal into a new ByteArray#.
A ByteArray# literal would be the correct thing to generate,
but these are not yet supported; see also #17747.

Somewhat surprisingly, this change results in a slight
reduction in compiler allocations, averaging around 0.5%
on ghc's compiler performance tests, including when compiling
programs that contain no bignum literals to begin with.
The specific cause of this has not been investigated.

Since this lowering no longer reads the interface file for
GHC.Num.BigNat, the reasoning in Note [Depend on GHC.Num.Integer]
is obsoleted.  But the story of un-tracked built-in dependencies
remains complex, and Note [Tracking dependencies on primitives]
now exists to explain this complexity.

Additionally, many empty imports have been modified to refer to
this new note and comply with its guidance.  Several empty imports
necessary for other reasons have also been given brief explanations.

Metric Decrease:
    MultiLayerModulesTH_OneShot

- - - - -
349ea330 by Fendor at 2024-03-19T14:50:00-04:00
Eliminate thunk in 'IfaceTyCon'

Heap analysis showed that `IfaceTyCon` retains a thunk to
`IfaceTyConInfo`, defeating the sharing of the most common instances of
`IfaceTyConInfo`.
We make sure the indirection is removed by adding bang patterns to
`IfaceTyCon`.

Experimental results on the agda code base, where the `mi_extra_decls`
were read from disk:

Before this change, we observe around 8654045 instances of:

`IfaceTyCon[Name,THUNK_1_0]`

But these thunks almost exclusively point to a shared value!

Forcing the thunk a little bit more, leads to `ghc-debug` reporting:

`IfaceTyCon[Name:Name,IfaceTyConInfo]`

and a noticeable reduction of live bytes (on agda ~10%).

- - - - -
594bee0b by Krzysztof Gogolewski at 2024-03-19T14:50:36-04:00
Minor misc cleanups

- GHC.HsToCore.Foreign.JavaScript: remove dropRuntimeRepArgs;
  boxed tuples don't take RuntimeRep args
- GHC.HsToCore.Foreign.Call: avoid partial pattern matching
- GHC.Stg.Unarise: strengthen the assertion; we can assert that
  non-rubbish literals are unary rather than just non-void
- GHC.Tc.Gen.HsType: make sure the fsLit "literal" rule fires
- users_guide/using-warnings.rst: remove -Wforall-identifier,
  now deprecated and does nothing
- users_guide/using.rst: fix formatting
- andy_cherry/test.T: remove expect_broken_for(23272...), 23272 is fixed

The rest are simple cleanups.

- - - - -
cf55a54b by Ben Gamari at 2024-03-19T14:51:12-04:00
mk/relpath: Fix quoting

Previously there were two instances in this script which lacked proper
quoting. This resulted in `relpath` invocations in the binary
distribution Makefile producing incorrect results on Windows, leading to
confusing failures from `sed` and the production of empty package
registrations.

Fixes #24538.

- - - - -
5ff88389 by Bryan Richter at 2024-03-19T14:51:48-04:00
testsuite: Disable T21336a on wasm

- - - - -
60023351 by Ben Gamari at 2024-03-19T22:33:10-04:00
hadrian/bindist: Eliminate extraneous `dirname` invocation

Previously we would call `dirname` twice per installed library file.
We now instead reuse this result. This helps appreciably on Windows, where
processes are quite expensive.

- - - - -
616ac300 by Ben Gamari at 2024-03-19T22:33:10-04:00
hadrian: Package mingw toolchain in expected location

This fixes #24525, a regression due to 41cbaf44a6ab5eb9fa676d65d32df8377898dc89.
Specifically, GHC expects to find the mingw32 toolchain in the binary distribution
root. However, after this patch it was packaged in the `lib/` directory.

- - - - -
de9daade by Ben Gamari at 2024-03-19T22:33:11-04:00
gitlab/rel_eng: More upload.sh tweaks

- - - - -
1dfe12db by Ben Gamari at 2024-03-19T22:33:11-04:00
rel_eng: Drop dead prepare_docs codepath

- - - - -
dd2d748b by Ben Gamari at 2024-03-19T22:33:11-04:00
rel_env/recompress_all: unxz before recompressing

Previously we would rather compress the xz *again*, before in addition
compressing it with the desired scheme.

Fixes #24545.

- - - - -
9d936c57 by Ben Gamari at 2024-03-19T22:33:11-04:00
mk-ghcup-metadata: Fix directory of testsuite tarball

As reported in #24546, the `dlTest` artifact should be extracted into
the `testsuite` directory.

- - - - -
6d398066 by Ben Gamari at 2024-03-19T22:33:11-04:00
ghcup-metadata: Don't populate dlOutput unless necessary

ghcup can apparently infer the output name of an artifact from its URL.
Consequently, we should only include the `dlOutput` field when it would
differ from the filename of `dlUri`.

Fixes #24547.

- - - - -
576f8b7e by Zubin Duggal at 2024-03-19T22:33:46-04:00
Revert "Apply shellcheck suggestion to SUBST_TOOLDIR"

This reverts commit c82770f57977a2b5add6e1378f234f8dd6153392.

The shellcheck suggestion is spurious and results in SUBST_TOOLDIR being a
no-op. `set` sets positional arguments for bash, but we want to set the variable
given as the first autoconf argument.

Fixes #24542

Metric decreases because the paths in the settings file are now shorter,
so we allocate less when we read the settings file.

-------------------------
Metric Decrease:
    T12425
    T13035
    T9198
-------------------------

- - - - -
cdfe6e01 by Fendor at 2024-03-19T22:34:22-04:00
Compact serialisation of IfaceAppArgs

In #24563, we identified that IfaceAppArgs serialisation tags each
cons cell element with a discriminator byte. These bytes add up
quickly, blowing up interface files considerably when
'-fwrite-if-simplified-core' is enabled.

We compact the serialisation by writing out the length of
'IfaceAppArgs', followed by serialising the elements directly without
any discriminator byte.

This improvement can decrease the size of some interface files by up
to 35%.

- - - - -
97a2bb1c by Simon Peyton Jones at 2024-03-20T17:11:29+00:00
Expand untyped splices in tcPolyExprCheck

Fixes #24559

- - - - -
5f275176 by Alan Zimmerman at 2024-03-20T22:44:12-04:00
EPA: Clean up Exactprint helper functions a bit

- Introduce a helper lens to compose on `EpAnn a` vs `a` versions
- Rename some prime versions of functions back to non-prime
  They were renamed during the rework

- - - - -
da2a10ce by Vladislav Zavialov at 2024-03-20T22:44:48-04:00
Type operators in promoteOccName (#24570)

Type operators differ from term operators in that they are lexically
classified as (type) constructors, not as (type) variables.

Prior to this change, promoteOccName did not account for this
difference, causing a scoping issue that affected RequiredTypeArguments.

  type (!@#) = Bool
  f = idee (!@#)      -- Not in scope: ‘!@#’  (BUG)

Now we have a special case in promoteOccName to account for this.

- - - - -
247fc0fa by Preetham Gujjula at 2024-03-21T10:19:18-04:00
docs: Remove mention of non-existent Ord instance for Complex

The documentation for Data.Complex says that the Ord instance for Complex Float
is deficient, but there is no Ord instance for Complex a. The Eq instance for
Complex Float is similarly deficient, so we use that as an example instead.

- - - - -
6fafc51e by Andrei Borzenkov at 2024-03-21T10:19:54-04:00
Fix TH handling in `pat_to_type_pat` function (#24571)

There was missing case for `SplicePat` in `pat_to_type_at` function,
hence patterns with splicing that checked against `forall->` doesn't work
properly because they fall into the "illegal pattern" case.

Code example that is now accepted:

  g :: forall a -> ()
  g $([p| a |]) = ()

- - - - -
52072f8e by Sylvain Henry at 2024-03-21T21:01:59-04:00
Type-check default declarations before deriving clauses (#24566)

See added Note and #24566. Default declarations must be type-checked
before deriving clauses.

- - - - -
7dfdf3d9 by Sylvain Henry at 2024-03-21T21:02:40-04:00
Lexer: small perf changes

- Use unsafeChr because we know our values to be valid
- Remove some unnecessary use of `ord` (return Word8 values directly)

- - - - -
864922ef by Sylvain Henry at 2024-03-21T21:02:40-04:00
JS: fix some comments

- - - - -
3e0b2b1f by Sebastian Graf at 2024-03-21T21:03:16-04:00
Simplifier: Re-do dependency analysis in abstractFloats (#24551)

In #24551, we abstracted a string literal binding over a type variable,
triggering a CoreLint error when that binding floated to top-level.

The solution implemented in this patch fixes this by re-doing dependency
analysis on a simplified recursive let binding that is about to be type
abstracted, in order to find the minimal set of type variables to abstract over.
See wrinkle (AB5) of Note [Floating and type abstraction] for more details.

Fixes #24551

- - - - -
c8470ff9 by Simon Peyton Jones at 2024-03-22T12:22:44+00:00
Rename tryCaseMerge

- - - - -
11ff00ab by Simon Peyton Jones at 2024-03-22T12:22:44+00:00
Try a more clever discard-eval

Addresses programs like this

  f xs = xs `seq`
         (let t = reverse $ reverse $ reverse $ reverse $ reverse $ reverse xs in
          case xs of
            [] ->  (t,True)
            (_:_) -> (t,False))

Also including the case where t is a join point.

Relates to #24251.  See GHC Log 13 March

- - - - -
b746fc5b by Simon Peyton Jones at 2024-03-22T12:22:44+00:00
Wibbles

- - - - -
88148701 by Simon Peyton Jones at 2024-03-22T14:24:11+00:00
Move to CSE

- - - - -


30 changed files:

- .gitlab/rel_eng/mk-ghcup-metadata/mk_ghcup_metadata.py
- .gitlab/rel_eng/recompress-all
- .gitlab/rel_eng/upload.sh
- compiler/GHC/Builtin/Names.hs
- compiler/GHC/Builtin/PrimOps.hs-boot
- compiler/GHC/Builtin/Types/Prim.hs
- compiler/GHC/Cmm/Dominators.hs
- compiler/GHC/Core/LateCC/OverloadedCalls.hs
- compiler/GHC/Core/Opt/CSE.hs
- compiler/GHC/Core/Opt/Simplify/Env.hs
- compiler/GHC/Core/Opt/Simplify/Iteration.hs
- compiler/GHC/Core/Opt/Simplify/Utils.hs
- compiler/GHC/Core/TyCo/Subst.hs
- compiler/GHC/CoreToStg/Prep.hs
- compiler/GHC/Data/Word64Map/Strict.hs
- compiler/GHC/Driver/CmdLine.hs
- compiler/GHC/Driver/Config/CoreToStg/Prep.hs
- compiler/GHC/Driver/Config/Diagnostic.hs
- compiler/GHC/Driver/Errors/Ppr.hs
- compiler/GHC/Driver/Main.hs
- compiler/GHC/Driver/Session.hs
- compiler/GHC/HsToCore/Expr.hs
- compiler/GHC/HsToCore/Foreign/Call.hs
- compiler/GHC/HsToCore/Foreign/JavaScript.hs
- compiler/GHC/Iface/Errors/Ppr.hs
- compiler/GHC/Iface/Syntax.hs
- compiler/GHC/Iface/Type.hs
- compiler/GHC/Iface/Type.hs-boot
- compiler/GHC/Parser.y
- compiler/GHC/Parser/Lexer.x


The diff was not included because it is too large.


View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/9975ad57752aacdf96eed7874c1a042bce3e05af...88148701841e8c6024fb22ba86b252ce95ffa850

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/9975ad57752aacdf96eed7874c1a042bce3e05af...88148701841e8c6024fb22ba86b252ce95ffa850
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20240322/83c72bf6/attachment-0001.html>


More information about the ghc-commits mailing list