[Git][ghc/ghc][wip/simplifier-tweaks] 40 commits: Fix sharing of 'IfaceTyConInfo' during core to iface type translation
Simon Peyton Jones (@simonpj)
gitlab at gitlab.haskell.org
Fri Mar 22 14:16:50 UTC 2024
Simon Peyton Jones pushed to branch wip/simplifier-tweaks 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
- - - - -
6b93843c by Simon Peyton Jones at 2024-03-22T09:43:18+00:00
Several improvements to the handling of coercions
* Make `mkSymCo` and `mkInstCo` smarter
Fixes #23642
* Fix return role of `SelCo` in the coercion optimiser.
Fixes #23617
* Make the coercion optimiser `opt_trans_rule` work better for newtypes
Fixes #23619
- - - - -
6a1bb930 by Simon Peyton Jones at 2024-03-22T14:16:28+00:00
FloatOut: improve floating for join point
See the new Note [Floating join point bindings].
* Completely get rid of the complicated join_ceiling nonsense, which
I have never understood.
* Do not float join points at all, except perhaps to top level.
* Some refactoring around wantToFloat, to treat Rec and NonRec more
uniformly
- - - - -
c8b16745 by Simon Peyton Jones at 2024-03-22T14:16:28+00:00
Improve eta-expansion through call stacks
See Note [Eta expanding through CallStacks] in GHC.Core.Opt.Arity
This is a one-line change, that fixes an inconsistency
- || isCallStackPredTy ty
+ || isCallStackPredTy ty || isCallStackTy ty
- - - - -
74b9abf3 by Simon Peyton Jones at 2024-03-22T14:16:28+00:00
Spelling, layout, pretty-printing only
- - - - -
20d2d798 by Simon Peyton Jones at 2024-03-22T14:16:28+00:00
Improve exprIsConApp_maybe a little
Eliminate a redundant case at birth. This sometimes reduces
Simplifier iterations.
See Note [Case elim in exprIsConApp_maybe].
- - - - -
08df237a by Simon Peyton Jones at 2024-03-22T14:16:28+00:00
Inline GHC.HsToCore.Pmc.Solver.Types.trvVarInfo
When exploring compile-time regressions after meddling with the Simplifier, I
discovered that GHC.HsToCore.Pmc.Solver.Types.trvVarInfo was very delicately
balanced. It's a small, heavily used, overloaded function and it's important
that it inlines. By a fluke it was before, but at various times in my journey it
stopped doing so. So I just added an INLINE pragma to it; no sense in depending
on a delicately-balanced fluke.
- - - - -
80c20812 by Simon Peyton Jones at 2024-03-22T14:16:28+00:00
Slight improvement in WorkWrap
Ensure that WorkWrap preserves lambda binders, in case of join points. Sadly I
have forgotten why I made this change (it was while I was doing a lot of
meddling in the Simplifier, but
* it does no harm,
* it is slightly more efficient, and
* presumably it made something better!
Anyway I have kept it in a separate commit.
- - - - -
3606be16 by Simon Peyton Jones at 2024-03-22T14:16:28+00:00
Use named record fields for the CastIt { ... } data constructor
This is a pure refactor
- - - - -
e442423a by Simon Peyton Jones at 2024-03-22T14:16:28+00:00
Remove a long-commented-out line
Pure refactoring
- - - - -
a83c5c34 by Simon Peyton Jones at 2024-03-22T14:16:28+00:00
Simplifier improvements
This MR started as: allow the simplifer to do more in one pass,
arising from places I could see the simplifier taking two iterations
where one would do. But it turned into a larger project, because
these changes unexpectedly made inlining blow up, especially join
points in deeply-nested cases.
The main changes are below. There are also many new or rewritten Notes.
Avoiding simplifying repeatedly
~~~~~~~~~~~~~~~
See Note [Avoiding simplifying repeatedly]
* The SimplEnv now has a seInlineDepth field, which says how deep
in unfoldings we are. See Note [Inline depth] in Simplify.Env.
Currently used only for the next point: avoiding repeatedly
simplifying coercions.
* Avoid repeatedly simplifying coercions.
see Note [Avoid re-simplifying coercions] in Simplify.Iteration
As you'll see from the Note, this makes use of the seInlineDepth.
* Allow Simplify.Iteration.simplAuxBind to inline used-once things.
This is another part of Note [Post-inline for single-use things], and
is really good for reducing simplifier iterations in situations like
case K e of { K x -> blah }
wher x is used once in blah.
* Make GHC.Core.SimpleOpt.exprIsConApp_maybe do some simple case
elimination. Note [Case elim in exprIsConApp_maybe]
postInlineUnconditionally
~~~~~~~~~~~~~~~~~~~~~~~~~
* Allow Simplify.Utils.postInlineUnconditionally to inline variables
that are used exactly once. See Note [Post-inline for single-use things].
* Do not postInlineUnconditionally join point, ever.
Doing so does not reduce allocation, which is the main point,
and with join points that are used a lot it can bloat code.
See point (1) of Note [Duplicating join points] in
GHC.Core.Opt.Simplify.Iteration.
* Do not postInlineUnconditionally a strict (demanded) binding.
It will not allocate a thunk (it'll turn into a case instead)
so again the main point of inlining it doesn't hold. Better
to check per-call-site.
* Improve occurrence analyis for bottoming function calls, to help
postInlineUnconditionally. See Note [Bottoming function calls]
in GHC.Core.Opt.OccurAnal
Inlining generally
~~~~~~~~~~~~~~~~~~
* In GHC.Core.Opt.Simplify.Utils.interestingCallContext,
use RhsCtxt NonRecursive (not BoringCtxt) for a plain-seq case.
See Note [Seq is boring]
* In GHC.Core.Opt.Simplify.Utils.interestingArg,
- return ValueArg for OtherCon [c1,c2, ...], but
- return NonTrivArg for OtherCon []
This makes a function a little less likely to inline if all we
know is that the argument is evaluated, but nothing else.
* isConLikeUnfolding is no longer true for OtherCon {}.
This propagates to exprIsConLike. Con-like-ness has /positive/
information.
Join points
~~~~~~~~~~~
* Be very careful about inlining join points.
See these two long Notes
Note [Duplicating join points] in GHC.Core.Opt.Simplify.Iteration
Note [Inlining join points] in GHC.Core.Opt.Simplify.Inline
* When making join points, don't do so if the join point is so small
it will immediately be inlined; check uncondInlineJoin.
* In GHC.Core.Opt.Simplify.Inline.tryUnfolding, improve the inlining
heuristics for join points. In general we /do not/ want to inline
join points /even if they are small/. See Note [Duplicating join points]
GHC.Core.Opt.Simplify.Iteration.
But sometimes we do: see Note [Inlining join points] in
GHC.Core.Opt.Simplify.Inline.
* Do not add an unfolding to a join point at birth. This is a tricky one
and has a long Note [Do not add unfoldings to join points at birth]
It shows up in two places
- In `mkDupableAlt` do not add an inlining
- (trickier) In `simplLetUnfolding` don't add an unfolding for a
fresh join point
I am not fully satisifed with this, but it works and is well documented.
* In GHC.Core.Unfold.sizeExpr, make jumps small, so that we don't penalise
having a non-inlined join point.
- - - - -
da989ef8 by Simon Peyton Jones at 2024-03-22T14:16:28+00:00
Testsuite message changes from simplifier improvements
- - - - -
22 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.hs
- compiler/GHC/Core/Coercion.hs
- compiler/GHC/Core/Coercion/Opt.hs
- compiler/GHC/Core/LateCC/OverloadedCalls.hs
- compiler/GHC/Core/Opt/Arity.hs
- compiler/GHC/Core/Opt/FloatOut.hs
- compiler/GHC/Core/Opt/Monad.hs
- compiler/GHC/Core/Opt/OccurAnal.hs
- compiler/GHC/Core/Opt/Pipeline.hs
- compiler/GHC/Core/Opt/SetLevels.hs
- compiler/GHC/Core/Opt/Simplify.hs
- compiler/GHC/Core/Opt/Simplify/Env.hs
- compiler/GHC/Core/Opt/Simplify/Inline.hs
- compiler/GHC/Core/Opt/Simplify/Iteration.hs
- compiler/GHC/Core/Opt/Simplify/Monad.hs
The diff was not included because it is too large.
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/401113c9731c7dcd47e5f2e39ffb6c5ee3975af4...da989ef8dc838a0a78e94c43a3d04e5bd9789e69
--
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/401113c9731c7dcd47e5f2e39ffb6c5ee3975af4...da989ef8dc838a0a78e94c43a3d04e5bd9789e69
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/b1185977/attachment-0001.html>
More information about the ghc-commits
mailing list