Skip to content
Snippets Groups Projects
  1. Aug 05, 2022
  2. Aug 04, 2022
    • Matthew Pickering's avatar
      Store interfaces in ModIfaceCache more directly · 1d94a59f
      Matthew Pickering authored and Marge Bot's avatar Marge Bot committed
      I realised hydration was completely irrelavant for this cache because
      the ModDetails are pruned from the result. So now it simplifies things a
      lot to just store the ModIface and Linkable, which we can put into the
      cache straight away rather than wait for the final version of a
      HomeModInfo to appear.
      1d94a59f
    • Matthew Pickering's avatar
      Fix leaks in --make mode when there are module loops · 0b1f5fd1
      Matthew Pickering authored and Marge Bot's avatar Marge Bot committed
      This patch fixes quite a tricky leak where we would end up retaining
      stale ModDetails due to rehydrating modules against non-finalised
      interfaces.
      
      == Loops with multiple boot files
      
      It is possible for a module graph to have a loop (SCC, when ignoring boot files)
      which requires multiple boot files to break. In this case we must perform the
      necessary hydration steps before and after compiling modules which have boot files
      which are described above for corectness but also perform an additional hydration step
      at the end of the SCC to remove space leaks.
      
      Consider the following example:
      
      ┌───────┐   ┌───────┐
      │       │   │       │
      │   A   │   │   B   │
      │       │   │       │
      └─────┬─┘   └───┬───┘
            │         │
       ┌────▼─────────▼──┐
       │                 │
       │        C        │
       └────┬─────────┬──┘
            │         │
       ┌────▼──┐  ┌───▼───┐
       │       │  │       │
       │ A-boot│  │ B-boot│
       │       │  │       │
       └───────┘  └───────┘
      
       A, B and C live together in a SCC. Say we compile the modules in order
       A-boot, B-boot, C, A, B then when we compile A we will perform the hydration steps
       (because A has a boot file). Therefore C will be hydrated relative to A, and the
       ModDetails for A will reference C/A. Then when B is compiled C will be rehydrated again,
       and so B will reference C/A,B, its interface will be hydrated relative to both A and B.
       Now there is a space leak because say C is a very big module, there are now two different copies of
       ModDetails kept alive by modules A and B.
      
      The way to avoid this space leak is to rehydrate an entire SCC together at the
      end of compilation so that all the ModDetails point to interfaces for .hs files.
      In this example, when we hydrate A, B and C together then both A and B will refer to
      C/A,B.
      
      See #21900 for some more discussion.
      
      -------------------------------------------------------
      
      In addition to this simple case, there is also the potential for a leak
      during parallel upsweep which is also fixed by this patch. Transcibed is
      Note [ModuleNameSet, efficiency and space leaks]
      
      Note [ModuleNameSet, efficiency and space leaks]
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      
      During unsweep the results of compiling modules are placed into a MVar, to find
      the environment the module needs to compile itself in the MVar is consulted and
      the HomeUnitGraph is set accordingly. The reason we do this is that precisely tracking
      module dependencies and recreating the HUG from scratch each time is very expensive.
      
      In serial mode (-j1), this all works out fine because a module can only be compiled after
      its dependencies have finished compiling and not interleaved with compiling module loops.
      Therefore when we create the finalised or no loop interfaces, the HUG only contains
      finalised interfaces.
      
      In parallel mode, we have to be more careful because the HUG variable can contain
      non-finalised interfaces which have been started by another thread. In order to avoid
      a space leak where a finalised interface is compiled against a HPT which contains a
      non-finalised interface we have to restrict the HUG to only the visible modules.
      
      The visible modules is recording in the ModuleNameSet, this is propagated upwards
      whilst compiling and explains which transitive modules are visible from a certain point.
      This set is then used to restrict the HUG before the module is compiled to only
      the visible modules and thus avoiding this tricky space leak.
      
      Efficiency of the ModuleNameSet is of utmost importance because a union occurs for
      each edge in the module graph. Therefore the set is represented directly as an IntSet
      which provides suitable performance, even using a UniqSet (which is backed by an IntMap) is
      too slow. The crucial test of performance here is the time taken to a do a no-op build in --make mode.
      
      See test "jspace" for an example which used to trigger this problem.
      
      Fixes #21900
      0b1f5fd1
    • Matthew Pickering's avatar
      Force name selectors to ensure no reference to Ids enter the NameCache · 0f43837f
      Matthew Pickering authored and Marge Bot's avatar Marge Bot committed
      I observed some unforced thunks in the NameCache which were retaining a
      whole Id, which ends up retaining a Type.. which ends up retaining old
      copies of HscEnv containing stale HomeModInfo.
      0f43837f
    • Matthew Pickering's avatar
      Force safeInferred to avoid retaining extra copy of DynFlags · fffc75a9
      Matthew Pickering authored and Marge Bot's avatar Marge Bot committed
      This will only have a (very) modest impact on memory but we don't want
      to retain old copies of DynFlags hanging around so best to force this
      value.
      fffc75a9
    • Andreas Klebinger's avatar
      Add a note about about W/W for unlifting strict arguments · fb529cae
      Andreas Klebinger authored and Marge Bot's avatar Marge Bot committed
      This fixes #21236.
      fb529cae
    • Krzysztof Gogolewski's avatar
      Fix TH + defer-type-errors interaction (#21920) · b99819bd
      Krzysztof Gogolewski authored and Marge Bot's avatar Marge Bot committed
      Previously, we had to disable defer-type-errors in splices because of #7276.
      But this fix is no longer necessary, the test T7276 no longer segfaults
      and is now correctly deferred.
      b99819bd
    • Yiyun Liu's avatar
      Remove TCvSubst and use Subst for both term and type-level subst · 35aef18d
      Yiyun Liu authored and Marge Bot's avatar Marge Bot committed
      This patch removes the TCvSubst data type and instead uses Subst as
      the environment for both term and type level substitution. This
      change is partially motivated by the existential type proposal,
      which will introduce types that contain expressions and therefore
      forces us to carry around an "IdSubstEnv" even when substituting for
      types. It also reduces the amount of code because "Subst" and
      "TCvSubst" share a lot of common operations. There isn't any
      noticeable impact on performance (geo. mean for ghc/alloc is around
      0.0% but we have -94 loc and one less data type to worry abount).
      
      Currently, the "TCvSubst" data type for substitution on types is
      identical to the "Subst" data type except the former doesn't store
      "IdSubstEnv". Using "Subst" for type-level substitution means there
      will be a redundant field stored in the data type. However, in cases
      where the substitution starts from the expression, using "Subst" for
      type-level substitution saves us from having to project "Subst" into a
      "TCvSubst". This probably explains why the allocation is mostly even
      despite the redundant field.
      
      The patch deletes "TCvSubst" and moves "Subst" and its relevant
      functions from "GHC.Core.Subst" into "GHC.Core.TyCo.Subst".
      Substitution on expressions is still defined in "GHC.Core.Subst" so we
      don't have to expose the definition of "Expr" in the hs-boot file that
      "GHC.Core.TyCo.Subst" must import to refer to "IdSubstEnv" (whose
      codomain is "CoreExpr"). Most functions named fooTCvSubst are renamed
      into fooSubst with a few exceptions (e.g. "isEmptyTCvSubst" is a
      distinct function from "isEmptySubst"; the former ignores the
      emptiness of "IdSubstEnv"). These exceptions mainly exist for
      performance reasons and will go away when "Expr" and "Type" are
      mutually recursively defined (we won't be able to take those
      shortcuts if we can't make the assumption that expressions don't
      appear in types).
      35aef18d
  3. Aug 02, 2022
  4. Jul 28, 2022
  5. Jul 26, 2022
    • Simon Peyton Jones's avatar
      Regression test for #21848 · 9ea29d47
      Simon Peyton Jones authored and Marge Bot's avatar Marge Bot committed
      9ea29d47
    • sterni's avatar
      hadrian: add flag disabling selftest rules which require QuickCheck · 42147534
      sterni authored and Marge Bot's avatar Marge Bot committed
      The hadrian executable depends on QuickCheck for building, meaning this
      library (and its dependencies) will need to be built for bootstrapping
      GHC in the future. Building QuickCheck, however, can require
      TemplateHaskell. When building a statically linking GHC toolchain,
      TemplateHaskell can be tricky to get to work, and cross-compiling
      TemplateHaskell doesn't work at all without -fexternal-interpreter,
      so QuickCheck introduces an element of fragility to GHC's bootstrap.
      
      Since the selftest rules are the only part of hadrian that need
      QuickCheck, we can easily eliminate this bootstrap dependency when
      required by introducing a `selftest` flag guarding the rules' inclusion.
      
      Closes #8699.
      42147534
    • Ben Gamari's avatar
      testsuite: Skip a few tests as in the nonmoving collector · 25c24535
      Ben Gamari authored and Marge Bot's avatar Marge Bot committed
      Residency monitoring under the non-moving collector is quite
      conservative (e.g. the reported value is larger than reality) since
      otherwise we would need to block on concurrent collection. Skip a few
      tests that are sensitive to residency.
      
      (cherry picked from commit 6880e4fb)
      25c24535
    • Ben Gamari's avatar
      rts/nonmoving: Don't scavenge objects which weren't evacuated · 54a5c32d
      Ben Gamari authored and Marge Bot's avatar Marge Bot committed
      This fixes a rather subtle bug in the logic responsible for scavenging
      objects evacuated to the non-moving generation. In particular, objects
      can be allocated into the non-moving generation by two ways:
      
       a. evacuation out of from-space by the garbage collector
       b. direct allocation by the mutator
      
      Like all evacuation, objects moved by (a) must be scavenged, since they
      may contain references to other objects located in from-space. To
      accomplish this we have the following scheme:
      
       * each nonmoving segment's block descriptor has a scan pointer which
         points to the first object which has yet to be scavenged
      
       * the GC tracks a set of "todo" segments which have pending scavenging
         work
      
       * to scavenge a segment, we scavenge each of the unmarked blocks
         between the scan pointer and segment's `next_free` pointer.
      
         We skip marked blocks since we know the allocator wouldn't have
         allocated into marked blocks (since they contain presumably live
         data).
      
         We can stop at `next_free` since, by
         definition, the GC could not have evacuated any objects to blocks
         above `next_free` (otherwise `next_free wouldn't be the first free
         block).
      
      However, this neglected to consider objects allocated by path (b).
      In short, the problem is that objects directly allocated by the mutator
      may become unreachable (but not swept, since the containing segment is
      not yet full), at which point they may contain references to swept objects.
      Specifically, we observed this in #21885 in the following way:
      
      1. the mutator (specifically in #21885, a `lockCAF`) allocates an object
         (specifically a blackhole, which here we will call `blkh`; see Note
         [Static objects under the nonmoving collector] for the reason why) on
         the non-moving heap. The bitmap of the allocated block remains 0
         (since allocation doesn't affect the bitmap) and the containing
         segment's (which we will call `blkh_seg`) `next_free` is advanced.
      2. We enter the blackhole, evaluating the blackhole to produce a result
         (specificaly a cons cell) in the nursery
      3. The blackhole gets updated into an indirection pointing to the cons
         cell; it is pushed to the generational remembered set
      4. we perform a GC, the cons cell is evacuated into the nonmoving heap
         (into segment `cons_seg`)
      5. the cons cell is marked
      6. the GC concludes
      7. the CAF and blackhole become unreachable
      8. `cons_seg` is filled
      9. we start another GC; the cons cell is swept
      10. we start a new GC
      11. something is evacuated into `blkh_seg`, adding it to the "todo" list
      12. we attempt to scavenge `blkh_seg` (namely, all unmarked blocks
          between `scan` and `next_free`, which includes `blkh`). We attempt to
          evacuate `blkh`'s indirectee, which is the previously-swept cons cell.
          This is unsafe, since the indirectee is no longer a valid heap
          object.
      
      The problem here was that the scavenging logic *assumed* that (a) was
      the only source of allocations into the non-moving heap and therefore
      *all* unmarked blocks between `scan` and `next_free` were evacuated.
      However, due to (b) this is not true.
      
      The solution is to ensure that that the scanned region only encompasses
      the region of objects allocated during evacuation. We do this by
      updating `scan` as we push the segment to the todo-segment list to
      point to the block which was evacuated into.
      
      Doing this required changing the nonmoving scavenging implementation's
      update of the `scan` pointer to bump it *once*, instead of after
      scavenging each block as was done previously. This is because we may end
      up evacuating into the segment being scavenged as we scavenge it. This
      was quite tricky to discover but the result is quite simple,
      demonstrating yet again that global mutable state should be used
      exceedingly sparingly.
      
      Fixes #21885
      
      (cherry picked from commit 0b27ea23)
      54a5c32d
    • Ben Gamari's avatar
      rts/nonmoving: Track segment state · 4b087973
      Ben Gamari authored and Marge Bot's avatar Marge Bot committed
      It can often be useful during debugging to be able to determine the
      state of a nonmoving segment. Introduce some state, enabled by DEBUG, to
      track this.
      
      (cherry picked from commit 40e797ef)
      4b087973
    • Ben Gamari's avatar
      testsuite: introduce nonmoving_thread_sanity way · 82a0991a
      Ben Gamari authored and Marge Bot's avatar Marge Bot committed
      (cherry picked from commit 19f8fce3)
      82a0991a
  6. Jul 25, 2022
    • Cheng Shao's avatar
      testsuite: Skip test cases involving -S when testing unregisterised GHC · 2869b66d
      Cheng Shao authored
      We no longer generate .s files anyway.
      
      Metric Decrease:
          MultiLayerModules
          T10421
          T13035
          T13701
          T14697
          T16875
          T18140
          T18304
          T18923
          T9198
      2869b66d
    • Cheng Shao's avatar
      Avoid as pipeline when compiling c · 96811ba4
      Cheng Shao authored and Cheng Shao's avatar Cheng Shao committed
      96811ba4
    • Cheng Shao's avatar
      Add location to cc phase · 4a7256a7
      Cheng Shao authored and Cheng Shao's avatar Cheng Shao committed
      4a7256a7
    • Simon Peyton Jones's avatar
      Get the in-scope set right in FamInstEnv.injectiveBranches · 61faff40
      Simon Peyton Jones authored and Marge Bot's avatar Marge Bot committed
      There was an assert error, as Gergo pointed out in #21896.
      
      I fixed this by adding an InScopeSet argument to tcUnifyTyWithTFs.
      And also to GHC.Core.Unify.niFixTCvSubst.
      
      I also took the opportunity to get a couple more InScopeSets right,
      and to change some substTyUnchecked into substTy.
      
      This MR touches a lot of other files, but only because I also took the
      opportunity to introduce mkInScopeSetList, and use it.
      61faff40
    • Simon Peyton Jones's avatar
      Add a 'notes' file in testsuite/tests/perf/compiler · 65f7838a
      Simon Peyton Jones authored and Marge Bot's avatar Marge Bot committed
      This file is just a place to accumlate notes about particular
      benchmarks, so that I don't keep re-inventing the wheel.
      65f7838a
    • Simon Peyton Jones's avatar
      Teach SpecConstr about typeDeterminesValue · d4fe2f4e
      Simon Peyton Jones authored and Marge Bot's avatar Marge Bot committed
      This patch addresses #21831, point 2.  See
      Note [generaliseDictPats] in SpecConstr
      
      I took the opportunity to refactor the construction of specialisation
      rules a bit, so that the rule name says what type we are specialising
      at.
      
      Surprisingly, there's a 20% decrease in compile time for test
      perf/compiler/T18223. I took a look at it, and the code size seems the
      same throughout. I did a quick ticky profile which seemed to show a
      bit less substitution going on.  Hmm.  Maybe it's the "don't do
      eta-expansion in stable unfoldings" patch, which is part of the
      same MR as this patch.
      
      Anyway, since it's a move in the right direction, I didn't think it
      was worth looking into further.
      
      Metric Decrease:
          T18223
      d4fe2f4e
    • Simon Peyton Jones's avatar
      Switch off eta-expansion in rules and unfoldings · 5d26c321
      Simon Peyton Jones authored and Marge Bot's avatar Marge Bot committed
      I think this change will make little difference except to reduce
      clutter.  But that's it -- if it causes problems we can switch it
      on again.
      5d26c321
    • Simon Peyton Jones's avatar
      Fix isEvaldUnfolding and isValueUnfolding · 726d938e
      Simon Peyton Jones authored and Marge Bot's avatar Marge Bot committed
      This fixes (1) in #21831.  Easy, obviously correct.
      726d938e
    • sterni's avatar
      ghc-cabal: allow Cabal 3.8 to unbreak make build · e4bf9592
      sterni authored and Marge Bot's avatar Marge Bot committed
      When bootstrapping GHC 9.4.*, the build will fail when configuring
      ghc-cabal as part of the make based build system due to this upper
      bound, as Cabal has been updated to a 3.8 release.
      
      Reference #21914, see especially
      ghc/ghc#21914 (comment 444699)
      e4bf9592
    • Simon Jakobi's avatar
      docs: Fix documentation of \cases · 79f1b021
      Simon Jakobi authored and Marge Bot's avatar Marge Bot committed
      Fixes #21902.
      79f1b021
    • Zubin's avatar
      Fix #21889, GHCi misbehaves with Ctrl-C on Windows · 3bbde957
      Zubin authored and Marge Bot's avatar Marge Bot committed
      On Windows, we create multiple levels of wrappers for GHCi which ultimately
      execute ghc --interactive. In order to handle console events properly, each of
      these wrappers must call FreeConsole() in order to hand off event processing to
      the child process. See #14150.
      
      In addition to this, FreeConsole must only be called from interactive processes (#13411).
      
      This commit makes two changes to fix this situation:
      
      1. The hadrian wrappers generated using `hadrian/bindist/cwrappers/version-wrapper.c` call `FreeConsole`
         if the CPP flag INTERACTIVE_PROCESS is set, which is set when we are generating a wrapper for GHCi.
      2. The GHCi wrapper in `driver/ghci/` calls the `ghc-$VER.exe` executable which is not wrapped rather
         than calling `ghc.exe` is is wrapped on windows (and usually non-interactive, so can't call `FreeConsole`:
      
         Before:
         ghci-$VER.exe calls ghci.exe which calls ghc.exe which calls ghc-$VER.exe
      
         After:
         ghci-$VER.exe calls ghci.exe which calls ghc-$VER.exe
      3bbde957
    • Simon Peyton Jones's avatar
      Fix a small buglet in tryEtaReduce · b77d95f8
      Simon Peyton Jones authored and Marge Bot's avatar Marge Bot committed
      Gergo points out (#21801) that GHC.Core.Opt.Arity.tryEtaReduce was
      making an ill-formed cast.  It didn't matter, because the subsequent
      guard discarded it; but still worth fixing.  Spurious warnings are
      distracting.
      b77d95f8
    • Simon Peyton Jones's avatar
      More improvements to worker/wrapper · 5f2fbd5e
      Simon Peyton Jones authored and Marge Bot's avatar Marge Bot committed
      This patch fixes #21888, and simplifies finaliseArgBoxities
      by eliminating the (recently introduced) data type FinalDecision.
      
      A delicate interaction meant that this patch
         commit d1c25a48
         Date:   Tue Jul 12 16:33:46 2022 +0100
         Refactor wantToUnboxArg a bit
      
      make worker/wrapper go into an infinite loop.  This patch
      fixes it by narrowing the handling of case (B) of
      Note [Boxity for bottoming functions], to deal only the
      arguemnts that are type variables.  Only then do we drop
      the trimBoxity call, which is what caused the bug.
      
      I also
      * Added documentation of case (B), which was previously
        completely un-mentioned.  And a regression test,
        T21888a, to test it.
      
      * Made unboxDeeplyDmd stop at lazy demands.  It's rare anyway
        for a bottoming function to have a lazy argument (mainly when
        the data type is recursive and then we don't want to unbox
        deeply).  Plus there is Note [No lazy, Unboxed demands in
        demand signature]
      
      * Refactored the Case equation for dmdAnal a bit, to do less
        redundant pattern matching.
      5f2fbd5e
    • sheaf's avatar
      Docs: clarify ConstraintKinds infelicity · c24ca5c3
      sheaf authored and Marge Bot's avatar Marge Bot committed
      GHC doesn't consistently require the ConstraintKinds extension to
      be enabled, as it allows programs such as type families returning
      a constraint without this extension.
      
      MR !7784 fixes this infelicity, but breaking user programs was deemed
      to not be worth it, so we document it instead.
      
      Fixes #21061.
      c24ca5c3
    • Bryan R's avatar
      ci: Disable (broken) perf-nofib · 73836fc8
      Bryan R authored and Marge Bot's avatar Marge Bot committed
      See #21859
      73836fc8
    • Gabriella439's avatar
      Default implementation for mempty/(<>) · 2a773259
      Gabriella439 authored and Marge Bot's avatar Marge Bot committed
      Approved by: https://github.com/haskell/core-libraries-committee/issues/61
      
      This adds a default implementation for `mempty` and `(<>)` along
      with a matching `MINIMAL` pragma so that `Semigroup` and `Monoid`
      instances can be defined in terms of `sconcat` / `mconcat`.
      
      The description for each class has also been updated to include the
      equivalent set of laws for the `sconcat`-only / `mconcat`-only
      instances.
      2a773259
    • Zubin's avatar
      Add DeepSubsumption09 · 918620d9
      Zubin authored and Marge Bot's avatar Marge Bot committed
      918620d9
    • Simon Peyton Jones's avatar
      Fix the interaction of operator sections and deep subsumption · 5e93a952
      Simon Peyton Jones authored and Marge Bot's avatar Marge Bot committed
      Fixes DeepSubsumption08
      5e93a952
    • Matthew Pickering's avatar
      Add DeepSubsumption08 · 67189985
      Matthew Pickering authored and Marge Bot's avatar Marge Bot committed
      67189985
Loading