1. 06 Sep, 2018 2 commits
    • Ömer Sinan Ağacan's avatar
      Fix a race between GC threads in concurrent scavenging · c6fbac6a
      Ömer Sinan Ağacan authored
      While debugging #15285 I realized that free block lists (free_list in
      BlockAlloc.c) get corrupted when multiple scavenge threads allocate and
      release blocks concurrently. Here's a picture of one such race:
          Thread 2 (Thread 32573.32601):
          #0  check_tail
              (bd=0x940d40 <stg_TSO_info>) at rts/sm/BlockAlloc.c:860
          #1  0x0000000000928ef7 in checkFreeListSanity
              () at rts/sm/BlockAlloc.c:896
          #2  0x0000000000928979 in freeGroup
              (p=0x7e998ce02880) at rts/sm/BlockAlloc.c:721
          #3  0x0000000000928a17 in freeChain
              (bd=0x7e998ce02880) at rts/sm/BlockAlloc.c:738
          #4  0x0000000000926911 in freeChain_sync
              (bd=0x7e998ce02880) at rts/sm/GCUtils.c:80
          #5  0x0000000000934720 in scavenge_capability_mut_lists
              (cap=0x1acae80) at rts/sm/Scav.c:1665
          #6  0x000000000092b411 in gcWorkerThread
              (cap=0x1acae80) at rts/sm/GC.c:1157
          #7  0x000000000090be9a in yieldCapability
              (pCap=0x7f9994e69e20, task=0x7e9984000b70, gcAllowed=true) at rts/Capability.c:861
          #8  0x0000000000906120 in scheduleYield
              (pcap=0x7f9994e69e50, task=0x7e9984000b70) at rts/Schedule.c:673
          #9  0x0000000000905500 in schedule
              (initialCapability=0x1acae80, task=0x7e9984000b70) at rts/Schedule.c:293
          #10 0x0000000000908d4f in scheduleWorker
              (cap=0x1acae80, task=0x7e9984000b70) at rts/Schedule.c:2554
          #11 0x000000000091a30a in workerStart
              (task=0x7e9984000b70) at rts/Task.c:444
          #12 0x00007f99937fa6db in start_thread
              (arg=0x7f9994e6a700) at pthread_create.c:463
          #13 0x000061654d59f88f in clone
              () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
          Thread 1 (Thread 32573.32573):
          #0  checkFreeListSanity
              () at rts/sm/BlockAlloc.c:887
          #1  0x0000000000928979 in freeGroup
              (p=0x7e998d303540) at rts/sm/BlockAlloc.c:721
          #2  0x0000000000926f23 in todo_block_full
              (size=513, ws=0x1aa8ce0) at rts/sm/GCUtils.c:264
          #3  0x00000000009583b9 in alloc_for_copy
              (size=513, gen_no=0) at rts/sm/Evac.c:80
          #4  0x000000000095850d in copy_tag_nolock
              (p=0x7e998c675f28, info=0x421d98 <Main_Large_con_info>, src=0x7e998d075d80, size=513,
              gen_no=0, tag=1) at rts/sm/Evac.c:153
          #5  0x0000000000959177 in evacuate
              (p=0x7e998c675f28) at rts/sm/Evac.c:715
          #6  0x0000000000932388 in scavenge_small_bitmap
              (p=0x7e998c675f28, size=1, bitmap=0) at rts/sm/Scav.c:271
          #7  0x0000000000934aaf in scavenge_stack
              (p=0x7e998c675f28, stack_end=0x7e998c676000) at rts/sm/Scav.c:1908
          #8  0x0000000000934295 in scavenge_one
              (p=0x7e998c66e000) at rts/sm/Scav.c:1466
          #9  0x0000000000934662 in scavenge_mutable_list
              (bd=0x7e998d300440, gen=0x1b1d880) at rts/sm/Scav.c:1643
          #10 0x0000000000934700 in scavenge_capability_mut_lists
              (cap=0x1aaa340) at rts/sm/Scav.c:1664
          #11 0x00000000009299b6 in GarbageCollect
              (collect_gen=0, do_heap_census=false, gc_type=2, cap=0x1aaa340, idle_cap=0x1b38aa0)
              at rts/sm/GC.c:378
          #12 0x0000000000907a4a in scheduleDoGC
              (pcap=0x7ffdec5b5310, task=0x1b36650, force_major=false) at rts/Schedule.c:1798
          #13 0x0000000000905de7 in schedule
              (initialCapability=0x1aaa340, task=0x1b36650) at rts/Schedule.c:546
          #14 0x0000000000908bc4 in scheduleWaitThread
              (tso=0x7e998c0067c8, ret=0x0, pcap=0x7ffdec5b5430) at rts/Schedule.c:2537
          #15 0x000000000091b5a0 in rts_evalLazyIO
              (cap=0x7ffdec5b5430, p=0x9c11f0, ret=0x0) at rts/RtsAPI.c:530
          #16 0x000000000091ca56 in hs_main
              (argc=1, argv=0x7ffdec5b5628, main_closure=0x9c11f0, rts_config=...) at rts/RtsMain.c:72
          #17 0x0000000000421ea0 in main
      In particular, dbl_link_onto() which is used to add a freed block to a
      doubly-linked free list is not thread safe and corrupts the list when
      called concurrently.
      Note that thread 1 is to blame here as thread 2 is properly taking the
      spinlock. With this patch we now take the spinlock when freeing a todo
      block in GC, avoiding this race.
      Test Plan:
      - Tried slow validate locally: this patch does not introduce new failures.
      - circleci: https://circleci.com/gh/ghc/ghc-diffs/283 The test got killed
        because it took 5 hours but T7919 (which was previously failing on circleci)
      Reviewers: simonmar, bgamari, erikd
      Reviewed By: simonmar
      Subscribers: rwbarton, carter
      GHC Trac Issues: #15285
      Differential Revision: https://phabricator.haskell.org/D5115
    • Ömer Sinan Ağacan's avatar
      Remove an incorrect assertion in threadPaused: · 16bc7ae8
      Ömer Sinan Ağacan authored
      The assertion is triggered when we have a loop in the program (in which case we
      see the same update frame multiple times in the stack). See #14915 for more
      Reviewers: simonmar, bgamari, erikd
      Reviewed By: simonmar
      Subscribers: rwbarton, carter
      GHC Trac Issues: #14915
      Differential Revision: https://phabricator.haskell.org/D5133
  2. 05 Sep, 2018 7 commits
    • Simon Peyton Jones's avatar
      Preserve specialisations despite CSE · 3addf72a
      Simon Peyton Jones authored
      Trac #15445 showed that, as a result of CSE, a function with an
      automatically generated specialisation RULE could be inlined
      before the RULE had a chance to fire.
      This patch attaches a NOINLINE[2] activation to the Id, during
      CSE, to stop this happening.
      See Note [Delay inlining after CSE]
      ---- Historical note ---
      This patch is simpler and more direct than an earlier
        commit 2110738b
        Author: Simon Peyton Jones <simonpj@microsoft.com>
        Date:   Mon Jul 30 13:43:56 2018 +0100
        Don't inline functions with RULES too early
      We had to revert this patch because it made GHC itself slower.
      Why? It delayed inlining of /all/ functions with RULES, and that was
      very bad in TcFlatten.flatten_ty_con_app
      * It delayed inlining of liftM
      * That delayed the unravelling of the recursion in some dictionary
      * That delayed some eta expansion, leaving
           flatten_ty_con_app = \x y. let <stuff> in \z. blah
      * That allowed the float-out pass to put sguff between
        the \y and \z.
      * And that permanently stopped eta expasion of the function,
        even once <stuff> was simplified.
      -- End of historical note ---
    • Simon Peyton Jones's avatar
      Define activeAfterInitial, activeDuringFinal · 1152a3be
      Simon Peyton Jones authored
      This is pure refactoring, just adding a couple of
      definitions to BasicTypes, and using them.
      Plus some whitespace stuff.
    • Alec Theriault's avatar
      Expose 'moduleToPkgConfAll' from 'PackageState' · e29ac2db
      Alec Theriault authored
      Having direct access to this field is going to enable Haddock to
      compute in batch which modules to load before looking up instances
      of external packages.
      Reviewers: bgamari, monoidal
      Reviewed By: monoidal
      Subscribers: rwbarton, carter
      Differential Revision: https://phabricator.haskell.org/D5100
    • Ben Gamari's avatar
      testsuite: Use bools for booleans, not ints · ecde9546
      Ben Gamari authored
      Summary: Just as it says on the tin.
      Test Plan: Validate
      Reviewers: bgamari, osa1
      Reviewed By: osa1
      Subscribers: osa1, monoidal, rwbarton, thomie, carter
      Differential Revision: https://phabricator.haskell.org/D5010
    • Chaitanya Koparkar's avatar
      base: Add references to Notes for certain special imports · a811d938
      Chaitanya Koparkar authored
      Modules like GHC.Integer, GHC.Natural etc. are special and sometimes
      have to be imported just to resolve build ordering issues. It's useful
      to refer to the appropriate Notes at such import sites.
      Test Plan: Read it.
      Reviewers: RyanGlScott, bgamari, hvr, simonpj
      Reviewed By: RyanGlScott, simonpj
      Subscribers: simonpj, rwbarton, carter
      GHC Trac Issues: #15526
      Differential Revision: https://phabricator.haskell.org/D5092
    • Ben Gamari's avatar
      testsuite: Add test for #15368 · 49d50b2b
      Ben Gamari authored
      Reviewers: bgamari, osa1
      Reviewed By: osa1
      Subscribers: osa1, monoidal, rwbarton, thomie, carter
      GHC Trac Issues: #15368
      Differential Revision: https://phabricator.haskell.org/D4958
    • Ömer Sinan Ağacan's avatar
      Skip eventlog tests in GHCi way · c0e5087d
      Ömer Sinan Ağacan authored
      Summary: (GHCi doesn't generate event logs)
      Test Plan:
      These tests were failing in GHCi way, they're now skipped in GHCi way as GHCi
      doesn't generate eventlogs
      Reviewers: bgamari, simonmar, maoe, alpmestan
      Reviewed By: alpmestan
      Subscribers: rwbarton, carter
      GHC Trac Issues: #15587
      Differential Revision: https://phabricator.haskell.org/D5119
  3. 04 Sep, 2018 5 commits
  4. 03 Sep, 2018 2 commits
  5. 02 Sep, 2018 2 commits
    • Alp Mestanogullari's avatar
      make iToBase62's inner loop stricter in one of its arguments · ed789516
      Alp Mestanogullari authored
      hadrian's support for dynamic ways is currently broken (see hadrian#641 [1]).
      The stage 1 GHCs that hadrian produces end up producing bad code for
      the `iToBase62` function after a few optimisation passes.
      In the case where `quotRem` returns (overflowError, 0),
      GHC isn't careful enough to realise q is _|_ and happily inlines,
      distributes and floats code around until we end up trying to access
      index `minBound :: Int` of an array of 62 chars, as a result of inlining
      the definition of `quotRem` for Ints, in particular the minBound branch [2].
      I will separately look into reproducing the bad transformation on a small
      self-contained example and filling a ticket.
      [1]: https://github.com/snowleopard/hadrian/issues/641
      [2]: https://git.haskell.org/ghc.git/blob/HEAD:/libraries/base/GHC/Real.hs#l366
      Test Plan: fixes hadrian#641
      Reviewers: bgamari, tdammers
      Reviewed By: tdammers
      Subscribers: tdammers, rwbarton, carter
      Differential Revision: https://phabricator.haskell.org/D5106
    • Ryan Scott's avatar
      Reject class instances with type families in kinds · 6dea7c16
      Ryan Scott authored
      GHC doesn't know how to handle type families that appear in
      class instances. Unfortunately, GHC didn't reject instances where
      type families appear in //kinds//, leading to #15515. This is easily
      rectified by calling `checkValidTypePat` on all arguments to a class
      in an instance (and not just the type arguments).
      Test Plan: make test TEST=T15515
      Reviewers: bgamari, goldfire, simonpj
      Reviewed By: simonpj
      Subscribers: simonpj, rwbarton, carter
      GHC Trac Issues: #15515
      Differential Revision: https://phabricator.haskell.org/D5068
  6. 31 Aug, 2018 4 commits
    • Simon Peyton Jones's avatar
      Remove knot-tying bug in TcHsSyn.zonkTyVarOcc · 565ef4cc
      Simon Peyton Jones authored
      There was a subtle knot-tying bug in TcHsSyn.zonkTyVarOcc, revealed
      in Trac #15552.
      I fixed it by
      * Eliminating the short-circuiting optimisation in zonkTyVarOcc,
        instead adding a finite map to get sharing of zonked unification
        See Note [Sharing when zonking to Type] in TcHsSyn
      * On the way I /added/ the short-circuiting optimisation to
        TcMType.zonkTcTyVar, which has no such problem.  This turned
        out (based on non-systematic measurements) to be a modest win.
        See Note [Sharing in zonking] in TcMType
      On the way I renamed some of the functions in TcHsSyn:
      * Ones ending in "X" (like zonkTcTypeToTypeX) take a ZonkEnv
      * Ones that do not end in "x" (like zonkTcTypeToType), don't.
        Instead they whiz up an empty ZonkEnv.
    • Simon Peyton Jones's avatar
      Commets on flatten_args_tc · fda2ea58
      Simon Peyton Jones authored
    • Simon Peyton Jones's avatar
      Comments only · 58518852
      Simon Peyton Jones authored
    • Richard Eisenberg's avatar
  7. 30 Aug, 2018 3 commits
  8. 29 Aug, 2018 3 commits
    • David Feuer's avatar
      Finish stable split · f48e276a
      David Feuer authored
      Long ago, the stable name table and stable pointer tables were one.
      Now, they are separate, and have significantly different
      implementations. I believe the time has come to finish the split
      that began in #7674.
      * Divide `rts/Stable` into `rts/StableName` and `rts/StablePtr`.
      * Give each table its own mutex.
      * Add FFI functions `hs_lock_stable_ptr_table` and
      `hs_unlock_stable_ptr_table` and document them.
        These are intended to replace the previously undocumented
      `hs_lock_stable_tables` and `hs_lock_stable_tables`,
        which are now documented as deprecated synonyms.
      * Make `eqStableName#` use pointer equality instead of unnecessarily
      comparing stable name table indices.
      Reviewers: simonmar, bgamari, erikd
      Reviewed By: bgamari
      Subscribers: rwbarton, carter
      GHC Trac Issues: #15555
      Differential Revision: https://phabricator.haskell.org/D5084
    • Andrey Mokhov's avatar
      Fix a constant folding rule · 65eec9cf
      Andrey Mokhov authored
      One of the constant folding rules introduced in D2858 is:
      (L y :-:   v) :-: (L x :-: w) -> return $ mkL (y-x)   `add` (w `add` v)
      Or, after removing syntactic noise: `(y - v) - (x - w) ==> (y - x) + (w + v)`.
      This is incorrect, since the sign of `v` is changed from negative to positive.
      As a consequence, the following program prints `3` when compiled with `-O`:
      -- This is just subtraction in disguise
      minus :: Int -> Int -> Int
      minus x y = (8 - y) - (8 - x)
      {-# NOINLINE minus #-}
      main :: IO ()
      main = print (2 `minus` 1)
      The correct rule is: `(y - v) - (x - w) ==> (y - x) + (w - v)`.
      This commit does the fix. I haven't found any other issues with the constant
      folding code, but it's difficult to be certain without some automated checking.
      Reviewers: bgamari, tdammers
      Subscribers: hsyl20, tdammers, rwbarton, carter
      GHC Trac Issues: #15569
      Differential Revision: https://phabricator.haskell.org/D5109
    • chris-bacon's avatar
      Fixed typo in exponent example · 36c1431d
      chris-bacon authored
  9. 28 Aug, 2018 4 commits
    • Ryan Scott's avatar
      Rename kind vars in left-to-right order in bindHsQTyVars · 102284e7
      Ryan Scott authored
      When renaming kind variables in an `LHsQTyVars`, we were
      erroneously putting all of the kind variables in the binders
      //after// the kind variables in the body, resulting in #15568. The
      fix is simple: just swap the order of these two around.
      Test Plan: make test TEST=T15568
      Reviewers: simonpj, bgamari, goldfire
      Reviewed By: goldfire
      Subscribers: goldfire, rwbarton, carter
      GHC Trac Issues: #15568
      Differential Revision: https://phabricator.haskell.org/D5108
    • Krzysztof Gogolewski's avatar
      Fix typo in 8.6.1 notes · 34b8e613
      Krzysztof Gogolewski authored
    • Ryan Scott's avatar
      Fix #15572 by checking for promoted names in ConT · c46a5f20
      Ryan Scott authored
      When converting `ConT`s to `HsTyVar`s in `Convert`, we were
      failing to account for the possibility of promoted data constructor
      names appearing in a `ConT`, which could result in improper
      pretty-printing results (as observed in #15572). The fix is
      straightforward: use `Promoted` instead of `NotPromoted` when the
      name of a `ConT` is a data constructor name.
      Test Plan: make test TEST=T15572
      Reviewers: goldfire, bgamari, simonpj, monoidal
      Reviewed By: goldfire, simonpj
      Subscribers: monoidal, rwbarton, carter
      GHC Trac Issues: #15572
      Differential Revision: https://phabricator.haskell.org/D5112
    • Krzysztof Gogolewski's avatar
      Remove dead code for commandline parsing · c18b525a
      Krzysztof Gogolewski authored
      PrefixPred and AnySuffixPred are not used
      since static flags were removed in bbd3c399.
      Test Plan: validate
      Reviewers: bgamari, tdammers
      Reviewed By: tdammers
      Subscribers: rwbarton, carter
      Differential Revision: https://phabricator.haskell.org/D5111
  10. 27 Aug, 2018 8 commits
    • Ben Gamari's avatar
      rts: Handle SMALL_MUT_ARR_PTRS in retainer profilter · 2cf98e22
      Ben Gamari authored
      Summary: These can be treated similarly to MUT_ARRY_PTRS. Fixes #15529.
      Reviewers: erikd, simonmar
      Reviewed By: simonmar
      Subscribers: RyanGlScott, rwbarton, carter
      GHC Trac Issues: #15529
      Differential Revision: https://phabricator.haskell.org/D5075
    • Chaitanya Koparkar's avatar
      Remove dph, vector, primitive and random from .gitmodules · 154d4e21
      Chaitanya Koparkar authored
      These packages were removed from the GHC source tree in
      Phab:D4761 and 0905fec0.
      Reviewers: RyanGlScott, bgamari
      Reviewed By: RyanGlScott
      Subscribers: rwbarton, carter
      Differential Revision: https://phabricator.haskell.org/D5095
    • Krzysztof Gogolewski's avatar
      Bump nofib submodule · b1f5d2fd
      Krzysztof Gogolewski authored
    • Chaitanya Koparkar's avatar
      Fix #10859 by using foldr1 while deriving Eq instances · 2d953a60
      Chaitanya Koparkar authored
      Previously, we were using foldl1 instead, which led to the derived
      code to be wrongly associated.
      Test Plan: ./validate
      Reviewers: RyanGlScott, nomeata, simonpj, bgamari
      Reviewed By: RyanGlScott, nomeata
      Subscribers: rwbarton, carter
      GHC Trac Issues: #10859
      Differential Revision: https://phabricator.haskell.org/D5104
    • Ryan Scott's avatar
      Don't reify redundant class method tyvars/contexts · 6e765aeb
      Ryan Scott authored
      Currently, reifying classes produces class methods with
      redundant tyvars and class contexts in their type signatures, such
      as in the following:
      class C a where
        method :: forall a. C a => a
      Fixing this is very straightforward: just apply `tcSplitMethodTy` to
      the type of each class method to lop off the redundant parts.
      It's possible that this could break some TH code in the wild that
      assumes the existence of these tyvars and class contexts, so I'll
      advertise this change in the release notes just to be safe.
      Test Plan: make test TEST="TH_reifyDecl1 T9064 T10891 T14888"
      Reviewers: goldfire, bgamari, simonpj
      Reviewed By: simonpj
      Subscribers: simonpj, rwbarton, carter
      GHC Trac Issues: #15551
      Differential Revision: https://phabricator.haskell.org/D5088
    • Ryan Scott's avatar
      Take strict fields into account in coverage checking · 744b034d
      Ryan Scott authored
      The current pattern-match coverage checker implements the
      formalism presented in the //GADTs Meet Their Match// paper in a
      fairly faithful matter. However, it was discovered recently that
      there is a class of unreachable patterns that
      //GADTs Meet Their Match// does not handle: unreachable code due to
      strict argument types, as demonstrated in #15305. This patch
      therefore goes off-script a little and implements an extension to
      the formalism presented in the paper to handle this case.
      Essentially, when determining if each constructor can be matched on,
      GHC checks if its associated term and type constraints are
      satisfiable. This patch introduces a new form of constraint,
      `NonVoid(ty)`, and checks if each constructor's strict argument types
      satisfy `NonVoid`. If any of them do not, then that constructor is
      deemed uninhabitable, and thus cannot be matched on. For the full
      story of how this works, see
      `Note [Extensions to GADTs Meet Their Match]`.
      Along the way, I did a little bit of much-needed refactoring. In
      particular, several functions in `Check` were passing a triple of
      `(ValAbs, ComplexEq, Bag EvVar)` around to represent a constructor
      and its constraints. Now that we're adding yet another form of
      constraint to the mix, I thought it appropriate to turn this into
      a proper data type, which I call `InhabitationCandidate`.
      Test Plan: make test TEST=T15305
      Reviewers: simonpj, bgamari
      Reviewed By: simonpj
      Subscribers: rwbarton, carter
      GHC Trac Issues: #15305
      Differential Revision: https://phabricator.haskell.org/D5087
    • Ryan Scott's avatar
      Fix #15502 by not casting to Int during TH conversion · 7a3cda53
      Ryan Scott authored
      When turning an `IntegerL` to an `IntegralLit` during TH
      conversion, we were stupidly casting an `Integer` to an `Int` in
      order to determine how it should be pretty-printed. Unsurprisingly,
      this causes problems when the `Integer` doesn't lie within the bounds
      of an `Int`, as demonstrated in #15502.
      The fix is simple: don't cast to an `Int`.
      Test Plan: make test TEST=T15502
      Reviewers: bgamari, simonpj
      Reviewed By: simonpj
      Subscribers: simonpj, rwbarton, carter
      GHC Trac Issues: #15502
      Differential Revision: https://phabricator.haskell.org/D5089
    • Ryan Scott's avatar
      Fix #15550 by quoting RULE names during TH conversion · 5e6cf2a9
      Ryan Scott authored
      When converting a `RuleP` to a GHC source `RuleD` during TH
      conversion, we were stupidly not double-quoting the name of the rule.
      Easily fixed.
      Test Plan: make test TEST=T15550
      Reviewers: goldfire, bgamari, simonpj
      Reviewed By: simonpj
      Subscribers: simonpj, rwbarton, carter
      GHC Trac Issues: #15550
      Differential Revision: https://phabricator.haskell.org/D5090