1. 22 Sep, 2020 4 commits
    • Sebastian Graf's avatar
      PmCheck: Rewrite inhabitation test · e9501547
      Sebastian Graf authored
      We used to produce inhabitants of a pattern-match refinement type Nabla
      in the checker in at least two different and mostly redundant ways:
      
        1. There was `provideEvidence` (now called
           `generateInhabitingPatterns`) which is used by
           `GHC.HsToCore.PmCheck` to produce non-exhaustive patterns, which
           produces inhabitants of a Nabla as a sub-refinement type where all
           match variables are instantiated.
        2. There also was `ensure{,All}Inhabited` (now called
           `inhabitationTest`) which worked slightly different, but was
           whenever new type constraints or negative term constraints were
           added. See below why `provideEvidence` and `ensureAllInhabited`
           can't be the same function, the main reason being performance.
        3. And last but not least there was the `nonVoid` test, which tested
           that a given type was inhabited. We did use this for strict fields
           and -XEmptyCase in the past.
      
      The overlap of (3) with (2) was always a major pet peeve of mine. The
      latter was quite efficient and proven to work for recursive data types,
      etc, but could not handle negative constraints well (e.g. we often want
      to know if a *refined* type is empty, such as `{ x:[a] | x /= [] }`).
      
      Lower Your Guards suggested that we could get by with just one, by
      replacing both functions with `inhabitationTest` in this patch.
      That was only possible by implementing the structure of φ constraints
      as in the paper, namely the semantics of φ constructor constraints.
      
      This has a number of benefits:
      
        a. Proper handling of unlifted types and strict fields, fixing #18249,
           without any code duplication between
           `GHC.HsToCore.PmCheck.Oracle.instCon` (was `mkOneConFull`) and
           `GHC.HsToCore.PmCheck.checkGrd`.
        b. `instCon` can perform the `nonVoid` test (3) simply by emitting
           unliftedness constraints for strict fields.
        c. `nonVoid` (3) is thus simply expressed by a call to
           `inhabitationTest`.
        d. Similarly, `ensureAllInhabited` (2), which we called after adding
           type info, now can similarly be expressed as the fuel-based
           `inhabitationTest`.
      
      See the new `Note [Why inhabitationTest doesn't call generateInhabitingPatterns]`
      why we still have tests (1) and (2).
      
      Fixes #18249 and brings nice metric decreases for `T17836` (-76%) and
      `T17836b` (-46%), as well as `T18478` (-8%) at the cost of a few very
      minor regressions (< +2%), potentially due to the fact that
      `generateInhabitingPatterns` does more work to suggest the minimal
      COMPLETE set.
      
      Metric Decrease:
          T17836
          T17836b
      e9501547
    • Sebastian Graf's avatar
      6fe8a0c7
    • Simon Peyton Jones's avatar
      Fix the occurrence analyser · 416bd50e
      Simon Peyton Jones authored
      Ticket #18603 demonstrated that the occurrence analyser's
      handling of
      
        local RULES for imported Ids
      
      (which I now call IMP-RULES) was inadequate.  It led the simplifier
      into an infnite loop by failing to label a binder as a loop breaker.
      
      The main change in this commit is to treat IMP-RULES in a simple and
      uniform way: as extra rules for the local binder.  See
        Note [IMP-RULES: local rules for imported functions]
      
      This led to quite a bit of refactoring.  The result is still tricky,
      but it's much better than before, and better documented I think.
      
      Oh, and it fixes the bug.
      416bd50e
    • Simon Peyton Jones's avatar
      Better eta-expansion (again) and don't specilise DFuns · 6de40f83
      Simon Peyton Jones authored
      This patch fixes #18223, which made GHC generate an exponential
      amount of code.  There are three quite separate changes in here
      
      1.  Re-engineer eta-expansion (again).  The eta-expander was
          generating lots of intermediate stuff, which could be optimised
          away, but which choked the simplifier meanwhile.  Relatively
          easy to kill it off at source.
      
          See Note [The EtaInfo mechanism] in GHC.Core.Opt.Arity.
          The main new thing is the use of pushCoArg in getArg_maybe.
      
      2.  Stop Specialise specalising DFuns.  This is the cause of a huge
          (and utterly unnecessary) blowup in program size in #18223.
          See Note [Do not specialise DFuns] in GHC.Core.Opt.Specialise.
      
          I also refactored the Specialise monad a bit... it was silly,
          because it passed on unchanging values as if they were mutable
          state.
      
      3.  Do an extra Simplifer run, after SpecConstra and before
          late-Specialise.  I found (investigating perf/compiler/T16473)
          that failing to do this was crippling *both* SpecConstr *and*
          Specialise.  See Note [Simplify after SpecConstr] in
          GHC.Core.Opt.Pipeline.
      
          This change does mean an extra run of the Simplifier, but only
          with -O2, and I think that's acceptable.
      
          T16473 allocates *three* times less with this change.  (I changed
          it to check runtime rather than compile time.)
      
      Some smaller consequences
      
      * I moved pushCoercion, pushCoArg and friends from SimpleOpt
        to Arity, because it was needed by the new etaInfoApp.
      
        And pushCoValArg now returns a MCoercion rather than Coercion for
        the argument Coercion.
      
      * A minor, incidental improvement to Core pretty-printing
      
      This does fix #18223, (which was otherwise uncompilable. Hooray.  But
      there is still a big intermediate because there are some very deeply
      nested types in that program.
      
      Modest reductions in compile-time allocation on a couple of benchmarks
          T12425     -2.0%
          T13253    -10.3%
      
      Metric increase with -O2, due to extra simplifier run
          T9233     +5.8%
          T12227    +1.8%
          T15630    +5.0%
      
      There is a spurious apparent increase on heap residency on T9630,
      on some architectures at least.  I tried it with -G1 and the residency
      is essentially unchanged.
      
      Metric Increase
          T9233
          T12227
          T9630
      
      Metric Decrease
          T12425
          T13253
      6de40f83
  2. 21 Sep, 2020 11 commits
  3. 19 Sep, 2020 17 commits
  4. 18 Sep, 2020 8 commits
    • Ben Gamari's avatar
      rts: Refactor unloading of foreign export StablePtrs · 40dc9106
      Ben Gamari authored
      Previously we would allocate a linked list cell for each foreign export.
      Now we can avoid this by taking advantage of the fact that they are
      already broken into groups.
      40dc9106
    • Ben Gamari's avatar
      rts: Refactor foreign export tracking · c4921349
      Ben Gamari authored
      This avoids calling `libc` in the initializers which are responsible for
      registering foreign exports. We believe this should avoid the corruption
      observed in #18548.
      
      See Note [Tracking foreign exports] in rts/ForeignExports.c for an
      overview of the new scheme.
      c4921349
    • Ben Gamari's avatar
      rts/nonmoving: Add missing STM write barrier · 0799b3de
      Ben Gamari authored
      When updating a TRec for a TVar already part of a transaction we
      previously neglected to add the old value to the update remembered set.
      I suspect this was the cause of #18587.
      0799b3de
    • Sylvain Henry's avatar
      b689f3db
    • Sylvain Henry's avatar
      Remove pprPrec from Outputable (unused) · 7f2785f2
      Sylvain Henry authored
      7f2785f2
    • Sylvain Henry's avatar
      Add note about OutputableP · 37aa224a
      Sylvain Henry authored
      37aa224a
    • Sylvain Henry's avatar
      Generalize OutputableP · e45c8544
      Sylvain Henry authored
      Add a type parameter for the environment required by OutputableP. It
      avoids tying Platform with OutputableP.
      e45c8544
    • Sylvain Henry's avatar
      Introduce OutputableP · ca48076a
      Sylvain Henry authored
      Some types need a Platform value to be pretty-printed: CLabel, Cmm
      types, instructions, etc.
      
      Before this patch they had an Outputable instance and the Platform value
      was obtained via sdocWithDynFlags. It meant that the *renderer* of the
      SDoc was responsible of passing the appropriate Platform value (e.g. via
      the DynFlags given to showSDoc).  It put the burden of passing the
      Platform value on the renderer while the generator of the SDoc knows the
      Platform it is generating the SDoc for and there is no point passing a
      different Platform at rendering time.
      
      With this patch, we introduce a new OutputableP class:
      
         class OutputableP a where
            pdoc :: Platform -> a -> SDoc
      
      With this class we still have some polymorphism as we have with `ppr`
      (i.e. we can use `pdoc` on a variety of types instead of having a
      dedicated `pprXXX` function for each XXX type).
      
      One step closer removing `sdocWithDynFlags` (#10143) and supporting
      several platforms (#14335).
      ca48076a