1. 06 Apr, 2020 3 commits
    • Simon Peyton Jones's avatar
      Refactoring only · e850d14f
      Simon Peyton Jones authored
      This refactors DictBinds into a data type rather than a pair.
      No change in behaviour, just better code
      e850d14f
    • Simon Peyton Jones's avatar
      Fix an tricky specialiser loop · cec2c71f
      Simon Peyton Jones authored
      Issue #17151 was a very tricky example of a bug in which the
      specialiser accidentally constructs a recurive dictionary,
      so that everything turns into bottom.
      
      I have fixed variants of this bug at least twice before:
      see Note [Avoiding loops].  It was a bit of a struggle
      to isolate the problem, greatly aided by the work that
      Alexey Kuleshevich did in distilling a test case.
      
      Once I'd understood the problem, it was not difficult to fix,
      though it did lead me a bit of refactoring in specImports.
      cec2c71f
    • Ömer Sinan Ağacan's avatar
      Don't override proc CafInfos in ticky builds · dcfe29c8
      Ömer Sinan Ağacan authored
      Fixes #17947
      
      When we have a ticky label for a proc, IdLabels for the ticky counter
      and proc entry share the same Name. This caused overriding proc CafInfos
      with the ticky CafInfos (i.e. NoCafRefs) during SRT analysis.
      
      We now ignore the ticky labels when building SRTMaps. This makes sense
      because:
      
      - When building the current module they don't need to be in SRTMaps as
        they're initialized as non-CAFFY (see mkRednCountsLabel), so they
        don't take part in the dependency analysis and they're never added to
        SRTs.
      
        (Reminder: a "dependency" in the SRT analysis is a CAFFY dependency,
        non-CAFFY uses are not considered as dependencies for the algorithm)
      
      - They don't appear in the interfaces as they're not exported, so it
        doesn't matter for cross-module concerns whether they're in the SRTMap
        or not.
      
      See also the new Note [Ticky labels in SRT analysis].
      dcfe29c8
  2. 04 Apr, 2020 1 commit
  3. 03 Apr, 2020 6 commits
    • Ömer Sinan Ağacan's avatar
      Revert accidental change in 9462452a · 40a85563
      Ömer Sinan Ağacan authored
      [ci skip]
      40a85563
    • Simon Peyton Jones's avatar
      Major improvements to the specialiser · 4291bdda
      Simon Peyton Jones authored
      This patch is joint work of Alexis King and Simon PJ.  It does some
      significant refactoring of the type-class specialiser.  Main highlights:
      
      * We can specialise functions with types like
           f :: Eq a => a -> Ord b => b => blah
        where the classes aren't all at the front (#16473).  Here we can
        correctly specialise 'f' based on a call like
           f @Int @Bool dEqInt x dOrdBool
        This change really happened in an earlier patch
           commit 2d0cf625
           Author: Sandy Maguire <sandy@sandymaguire.me>
           Date:   Thu May 16 12:12:10 2019 -0400
        work that this new patch builds directly on that work, and refactors
        it a bit.
      
      * We can specialise functions with implicit parameters (#17930)
           g :: (?foo :: Bool, Show a) => a -> String
        Previously we could not, but now they behave just like a non-class
        argument as in 'f' above.
      
      * We can specialise under-saturated calls, where some (but not all of
        the dictionary arguments are provided (#17966).  For example, we can
        specialise the above 'f' based on a call
           map (f @Int dEqInt) xs
        even though we don't (and can't) give Ord dictionary.
      
        This may sound exotic, but #17966 is a program from the wild, and
        showed significant perf loss for functions like f, if you need
        saturation of all dictionaries.
      
      * We fix a buglet in which a floated dictionary had a bogus demand
        (#17810), by using zapIdDemandInfo in the NonRec case of specBind.
      
      * A tiny side benefit: we can drop dead arguments to specialised
        functions; see Note [Drop dead args from specialisations]
      
      * Fixed a bug in deciding what dictionaries are "interesting"; see
        Note [Keep the old dictionaries interesting]
      
      This is all achieved by by building on Sandy Macguire's work in
      defining SpecArg, which mkCallUDs uses to describe the arguments of
      the call. Main changes:
      
      * Main work is in specHeader, which marched down the [InBndr] from the
        function definition and the [SpecArg] from the call site, together.
      
      * specCalls no longer has an arity check; the entire mechanism now
        handles unders-saturated calls fine.
      
      * mkCallUDs decides on an argument-by-argument basis whether to
        specialise a particular dictionary argument; this is new.
        See mk_spec_arg in mkCallUDs.
      
      It looks as if there are many more lines of code, but I think that
      all the extra lines are comments!
      4291bdda
    • Sylvain Henry's avatar
      Refactor CmmStatics · cc2918a0
      Sylvain Henry authored
      In !2959 we noticed that there was some redundant code (in GHC.Cmm.Utils
      and GHC.Cmm.StgToCmm.Utils) used to deal with `CmmStatics` datatype
      (before SRT generation) and `RawCmmStatics` datatype (after SRT
      generation).
      
      This patch removes this redundant code by using a single GADT for
      (Raw)CmmStatics.
      cc2918a0
    • Sylvain Henry's avatar
      Move blob handling into StgToCmm · a485c3c4
      Sylvain Henry authored
      Move handling of big literal strings from CmmToAsm to StgToCmm. It
      avoids the use of `sdocWithDynFlags` (cf #10143). We might need to move
      this handling even higher in the pipeline in the future (cf #17960):
      this patch will make it easier.
      a485c3c4
    • Andreas Klebinger's avatar
      Improve and refactor StgToCmm codegen for DataCons. · 9462452a
      Andreas Klebinger authored
      We now differentiate three cases of constructor bindings:
      
      1)Bindings which we can "replace" with a reference to
        an existing closure. Reference the replacement closure
        when accessing the binding.
      2)Bindings which we can "replace" as above. But we still
        generate a closure which will be referenced by modules
        importing this binding.
      3)For any other binding generate a closure. Then reference
        it.
      
      Before this patch 1) did only apply to local bindings and we
      didn't do 2) at all.
      9462452a
    • wz1000's avatar
      Add outputable instances for the types in GHC.Iface.Ext.Types, add -ddump-hie · ef7576c4
      wz1000 authored
      flag to dump pretty printed contents of the .hie file
      
      Metric Increase:
         hie002
      
      Because of the regression on i386:
      
      compile_time/bytes allocated increased from i386-linux-deb9 baseline @ HEAD~10:
          Expected    hie002 (normal) compile_time/bytes allocated: 583014888.0 +/-10%
          Lower bound hie002 (normal) compile_time/bytes allocated:   524713399
          Upper bound hie002 (normal) compile_time/bytes allocated:   641316377
          Actual      hie002 (normal) compile_time/bytes allocated:   877986292
          Deviation   hie002 (normal) compile_time/bytes allocated:        50.6 %
      *** unexpected stat test failure for hie002(normal)
      ef7576c4
  4. 02 Apr, 2020 4 commits
    • Ryan Scott's avatar
      Fix two ASSERT buglets in reifyDataCon · 30a63e79
      Ryan Scott authored
      Two `ASSERT`s in `reifyDataCon` were always using `arg_tys`, but
      `arg_tys` is not meaningful for GADT constructors. In fact, it's
      worse than non-meaningful, since using `arg_tys` when reifying a
      GADT constructor can lead to failed `ASSERT`ions, as #17305
      demonstrates.
      
      This patch applies the simplest possible fix to the immediate
      problem. The `ASSERT`s now use `r_arg_tys` instead of `arg_tys`, as
      the former makes sure to give something meaningful for GADT
      constructors. This makes the panic go away at the very least. There
      is still an underlying issue with the way the internals of
      `reifyDataCon` work, as described in
      #17305 (comment 227023), but we
      leave that as future work, since fixing the underlying issue is
      much trickier (see
      #17305 (comment 227087)).
      30a63e79
    • Ben Gamari's avatar
      Session: Memoize stderrSupportsAnsiColors · 88f38b03
      Ben Gamari authored
      Not only is this a reasonable efficiency measure but it avoids making
      reentrant calls into ncurses, which is not thread-safe. See #17922.
      88f38b03
    • Sebastian Graf's avatar
      Preserve precise exceptions in strictness analysis · 42d68364
      Sebastian Graf authored
      Fix #13380 and #17676 by
      
      1. Changing `raiseIO#` to have `topDiv` instead of `botDiv`
      2. Give it special treatment in `Simplifier.Util.mkArgInfo`, treating it
         as if it still had `botDiv`, to recover dead code elimination.
      
      This is the first commit of the plan outlined in
      !2525 (comment 260886).
      42d68364
    • Simon Peyton Jones's avatar
      Re-engineer the binder-swap transformation · b943b25d
      Simon Peyton Jones authored
      The binder-swap transformation is implemented by the occurrence
      analyser -- see Note [Binder swap] in OccurAnal. However it had
      a very nasty corner in it, for the case where the case scrutinee
      was a GlobalId.  This led to trouble and hacks, and ultimately
      to #16296.
      
      This patch re-engineers how the occurrence analyser implements
      the binder-swap, by actually carrying out a substitution rather
      than by adding a let-binding.  It's all described in
      Note [The binder-swap substitution].
      
      I did a few other things along the way
      
      * Fix a bug in StgCse, which could allow a loop breaker to be CSE'd
        away.  See Note [Care with loop breakers] in StgCse.  I think it can
        only show up if occurrence analyser sets up bad loop breakers, but
        still.
      
      * Better commenting in SimplUtils.prepareAlts
      
      * A little refactoring in CoreUnfold; nothing significant
        e.g. rename CoreUnfold.mkTopUnfolding to mkFinalUnfolding
      
      * Renamed CoreSyn.isFragileUnfolding to hasCoreUnfolding
      
      * Move mkRuleInfo to CoreFVs
      
      We observed respectively 4.6% and 5.9% allocation decreases for the following
      tests:
      
      Metric Decrease:
          T9961
          haddock.base
      b943b25d
  5. 01 Apr, 2020 3 commits
    • Sebastian Graf's avatar
      PmCheck: Adjust recursion depth for inhabitation test · 7b217179
      Sebastian Graf authored
      In #17977, we ran into the reduction depth limit of the typechecker.
      That was only a symptom of a much broader issue: The recursion depth
      of the coverage checker for trying to instantiate strict fields in the
      `nonVoid` test was far too high (100, the `defaultMaxTcBound`).
      
      As a result, we were performing quite poorly on `T17977`.
      Short of a proper termination analysis to prove emptyness of a type,
      we just arbitrarily default to a much lower recursion limit of 3.
      
      Fixes #17977.
      7b217179
    • Sylvain Henry's avatar
      Kill wORDS_BIGENDIAN and replace it with platformByteOrder (#17957) · 0002db1b
      Sylvain Henry authored
      Metric Decrease:
          T13035
          T1969
      0002db1b
    • Ryan Scott's avatar
      Clean up "Eta reduction for data families" Notes · 9b39f2e6
      Ryan Scott authored
      Before, there were two distinct Notes named
      "Eta reduction for data families". This renames one of them to
      "Implementing eta reduction for data families" to disambiguate the
      two and fixes references in other parts of the codebase to ensure
      that they are pointing to the right place.
      
      Fixes #17313.
      
      [ci skip]
      9b39f2e6
  6. 31 Mar, 2020 2 commits
  7. 29 Mar, 2020 7 commits
    • Krzysztof Gogolewski's avatar
      Minor cleanup · 45eb9d8c
      Krzysztof Gogolewski authored
      - Simplify mkBuildExpr, the function newTyVars was called
        only on a one-element list.
      - TTG: use noExtCon in more places. This is more future-proof.
      - In zonkExpr, panic instead of printing a warning.
      45eb9d8c
    • Ryan Scott's avatar
      Run checkNewDataCon before constraint-solving newtype constructors · a0d8e92e
      Ryan Scott authored
      Within `checkValidDataCon`, we used to run `checkValidType` on the
      argument types of a newtype constructor before running
      `checkNewDataCon`, which ensures that the user does not attempt
      non-sensical things such as newtypes with multiple arguments or
      constraints. This works out in most situations, but this falls over
      on a corner case revealed in #17955:
      
      ```hs
      newtype T = Coercible () T => T ()
      ```
      
      `checkValidType`, among other things, peforms an ambiguity check on
      the context of a data constructor, and that it turn invokes the
      constraint solver. It turns out that there is a special case in the
      constraint solver for representational equalities (read: `Coercible`
      constraints) that causes newtypes to be unwrapped (see
      `Note [Unwrap newtypes first]` in `TcCanonical`). This special case
      does not know how to cope with an ill formed newtype like `T`, so
      it ends up panicking.
      
      The solution is surprisingly simple: just invoke `checkNewDataCon`
      before `checkValidType` to ensure that the illicit newtype
      constructor context is detected before the constraint solver can
      run amok with it.
      
      Fixes #17955.
      a0d8e92e
    • Sylvain Henry's avatar
      Store ComponentId details · e54500c1
      Sylvain Henry authored
      As far as GHC is concerned, installed package components ("units") are
      identified by an opaque ComponentId string provided by Cabal. But we
      don't want to display it to users (as it contains a hash) so GHC queries
      the database to retrieve some infos about the original source package
      (name, version, component name).
      
      This patch caches these infos in the ComponentId itself so that we don't
      need to provide DynFlags (which contains installed package informations)
      to print a ComponentId.
      
      In the future we want GHC to support several independent package states
      (e.g. for plugins and for target code), hence we need to avoid
      implicitly querying a single global package state.
      e54500c1
    • Simon Peyton Jones's avatar
      Demand analysis: simplify the demand for a RHS · 54250f2d
      Simon Peyton Jones authored
      Ticket #17932 showed that we were using a stupid demand for the RHS
      of a let-binding, when the result is a product.  This was the result
      of a "fix" in 2013, which (happily) turns out to no longer be
      necessary.
      
      So I just deleted the code, which simplifies the demand analyser,
      and fixes #17932. That in turn uncovered that the anticipation
      of worker/wrapper in CPR analysis was inaccurate, hence the logic
      that decides whether to unbox an argument in WW was extracted into
      a function `wantToUnbox`, now consulted by CPR analysis.
      
      I tried nofib, and got 0.0% perf changes.
      
      All this came up when messing about with !2873 (ticket #17917),
      but is idependent of it.
      
      Unfortunately, this patch regresses #4267 and realised that it is now
      blocked on #16335.
      54250f2d
    • Sylvain Henry's avatar
    • Sylvain Henry's avatar
      Remove GHC.Types.Unique.Map module · 1c7c6f1a
      Sylvain Henry authored
      This module isn't used anywhere in GHC.
      1c7c6f1a
    • Sylvain Henry's avatar
      Modules: Types (#13009) · 1941ef4f
      Sylvain Henry authored
      Update Haddock submodule
      
      Metric Increase:
         haddock.compiler
      1941ef4f
  8. 26 Mar, 2020 4 commits
  9. 25 Mar, 2020 2 commits
    • Sebastian Graf's avatar
      Remove -fkill-absence and -fkill-one-shot flags · 3e27205a
      Sebastian Graf authored
      They seem to be a benchmarking vestige of the Cardinality paper and
      probably shouldn't have been merged to HEAD in the first place.
      3e27205a
    • Roland Senn's avatar
      Use export list of Main module in function TcRnDriver.hs:check_main (Fix #16453) · 703221f4
      Roland Senn authored
      - Provide the export list of the `Main` module as parameter to the
        `compiler/typecheck/TcRnDriver.hs:check_main` function.
      - Instead of `lookupOccRn_maybe` call the function `lookupInfoOccRn`.
        It returns the list `mains_all` of all the main functions in scope.
      - Select from this list `mains_all` all `main` functions that are in
        the export list of the `Main` module.
      - If this new list contains exactly one single `main` function, then
        typechecking continues.
      - Otherwise issue an appropriate error message.
      703221f4
  10. 23 Mar, 2020 1 commit
    • Josef Svenningsson's avatar
      Fix ApplicativeDo regression #17835 · 19f12557
      Josef Svenningsson authored
      A previous fix for #15344 made sure that monadic 'fail' is used properly
      when translating ApplicativeDo. However, it didn't properly account
      for when a 'fail' will be inserted which resulted in some programs
      failing with a type error.
      19f12557
  11. 21 Mar, 2020 3 commits
    • Sergej Jaskiewicz's avatar
      Fix event message in withTiming' · 7e0451c6
      Sergej Jaskiewicz authored
      This typo caused generating 'end' events without the corresponding 'begin' events.
      7e0451c6
    • Richard Eisenberg's avatar
      Update core spec to reflect changes to Core. · 9a96ff6b
      Richard Eisenberg authored
      Key changes:
       * Adds a new rule for forall-coercions over coercion variables, which
      was implemented but conspicuously missing from the spec.
       * Adds treatment for FunCo.
       * Adds treatment for ForAllTy over coercion variables.
       * Improves commentary (including restoring a Note lost in
      03d48526) in the source.
      
      No changes to running code.
      9a96ff6b
    • Richard Eisenberg's avatar
      Simplify treatment of heterogeneous equality · 73a7383e
      Richard Eisenberg authored
      Previously, if we had a [W] (a :: k1) ~ (rhs :: k2), we would
      spit out a [D] k1 ~ k2 and part the W as irreducible, hoping for
      a unification. But we needn't do this. Instead, we now spit out
      a [W] co :: k2 ~ k1 and then use co to cast the rhs of the original
      Wanted. This means that we retain the connection between the
      spat-out constraint and the original.
      
      The problem with this new approach is that we cannot use the
      casted equality for substitution; it's too like wanteds-rewriting-
      wanteds. So, we forbid CTyEqCans that mention coercion holes.
      
      All the details are in Note [Equalities with incompatible kinds]
      in TcCanonical.
      
      There are a few knock-on effects, documented where they occur.
      
      While debugging an error in this patch, Simon and I ran into
      infelicities in how patterns and matches are printed; we made
      small improvements.
      
      This patch includes mitigations for #17828, which causes spurious
      pattern-match warnings. When #17828 is fixed, these lines should
      be removed.
      73a7383e
  12. 19 Mar, 2020 3 commits
    • Ömer Sinan Ağacan's avatar
      FastString: fix eager reading of string ptr in hashStr · cb1785d9
      Ömer Sinan Ağacan authored
      This read causes NULL dereferencing when len is 0.
      
      Fixes #17909
      
      In the reproducer in #17909 this bug is triggered as follows:
      
      - SimplOpt.dealWithStringLiteral is called with a single-char string
        ("=" in #17909)
      
      - tailFS gets called on the FastString of the single-char string.
      
      - tailFS checks the length of the string, which is 1, and calls
        mkFastStringByteString on the tail of the ByteString, which is an
        empty ByteString as the original ByteString has only one char.
      
      - ByteString's unsafeUseAsCStringLen returns (NULL, 0) for the empty
        ByteString, which is passed to mkFastStringWith.
      
      - mkFastStringWith gets hash of the NULL pointer via hashStr, which
        fails on empty strings because of this bug.
      cb1785d9
    • Sylvain Henry's avatar
      Refactoring: use Platform instead of DynFlags when possible · 64f20756
      Sylvain Henry authored
      Metric Decrease:
          ManyConstructors
          T12707
          T13035
          T1969
      64f20756
    • Sebastian Graf's avatar
      PmCheck: Use ConLikeSet to model negative info · b03fd3bc
      Sebastian Graf authored
      In #17911, Simon recognised many warnings stemming from over-long list
      unions while coverage checking Cabal's `LicenseId` module.
      
      This patch introduces a new `PmAltConSet` type which uses a `UniqDSet`
      instead of an association list for `ConLike`s. For `PmLit`s, it will
      still use an assocation list, though, because a similar map data
      structure would entail a lot of busy work.
      
      Fixes #17911.
      b03fd3bc
  13. 18 Mar, 2020 1 commit