[Git][ghc/ghc][wip/T17676] 70 commits: Be explicit about how stack usage of mvar primops are covered.

Sebastian Graf gitlab at gitlab.haskell.org
Tue Mar 17 19:04:40 UTC 2020



Sebastian Graf pushed to branch wip/T17676 at Glasgow Haskell Compiler / GHC


Commits:
8c663c2c by Andreas Klebinger at 2020-03-04T15:12:14Z
Be explicit about how stack usage of mvar primops are covered.

This fixes #17893

[skip-ci]

- - - - -
cedd6f30 by Ben Gamari at 2020-03-05T19:53:12Z
rts: Add getCurrentThreadCPUTime helper

- - - - -
ace618cd by Ben Gamari at 2020-03-05T19:53:12Z
nonmoving-gc: Track time usage of nonmoving marking

- - - - -
022b5ad5 by Ben Gamari at 2020-03-05T19:53:12Z
Stats: Add sync pauses to +RTS -S output

- - - - -
06763234 by Ben Gamari at 2020-03-05T19:53:12Z
rts: Report nonmoving collector statistics in machine-readable output

- - - - -
70d2b995 by Ben Gamari at 2020-03-09T10:10:52Z
nonmoving: Fix collection of sparks

Previously sparks living in the non-moving heap would be promptly GC'd
by the minor collector since pruneSparkQueue uses the BF_EVACUATED flag,
which non-moving heap blocks do not have set.

Fix this by implementing proper support in pruneSparkQueue for
determining reachability in the non-moving heap. The story is told in
Note [Spark management in the nonmoving heap].

- - - - -
9668781a by Ben Gamari at 2020-03-09T10:11:30Z
gitlab-ci: Disable Sphinx documentation in Alpine build
- - - - -
8eb2c263 by Jean-Baptiste Mazon at 2020-03-09T20:33:37Z
Fix Windows breakage by not touching locales on Windows

- - - - -
b8dab057 by Jean-Baptiste Mazon at 2020-03-09T20:33:37Z
rts: ensure C numerics in heap profiles using Windows locales if needed

- - - - -
7d95260f by Jean-Baptiste Mazon at 2020-03-09T20:33:37Z
rts: refactor and comment profile locales

- - - - -
5b627813 by Ryan Scott at 2020-03-09T20:34:14Z
Use InstanceSigs in GND/DerivingVia-generated code (#17899)

Aside from making the generated code easier to read when
`-ddump-deriv` is enabled, this makes the error message in `T15073`
substantially simpler (see the updated `T15073` expected stderr).

Fixes #17899.

- - - - -
70b50778 by Ben Gamari at 2020-03-10T06:05:42Z
SysTools: Ensure that error parser can handle absolute paths on Windows

This fixes #17786, where the error parser fails to correctly handle the
drive name in absolute Windows paths.

Unfortunately I couldn't find a satisfactory way to test this.

- - - - -
85b861d8 by Ben Gamari at 2020-03-10T06:05:42Z
testsuite: Add test for #17786

This isn't pretty but it's perhaps better than nothing.

- - - - -
ee2c50cb by Sylvain Henry at 2020-03-10T06:06:33Z
Hadrian: track missing configure results

- - - - -
ca8f51d4 by Ömer Sinan Ağacan at 2020-03-10T06:07:22Z
Add regression test for T17904

Closes #17904

- - - - -
5fa9cb82 by Richard Eisenberg at 2020-03-10T16:29:46Z
anyRewritableTyVar now looks in RuntimeReps

Previously, anyRewritableTyVar looked only at the arg and res
of `arg -> res`, but their RuntimeReps are also subject to
rewriting. Easy to fix.

Test case: typecheck/should_compile/T17024

Fixes #17024.

- - - - -
5ba01d83 by Ben Price at 2020-03-10T16:30:27Z
Clarify a Lint message

When developing a plugin I had a shadowing problem, where I generated
code
  app = \f{v r7B} x{v r7B} -> f{v r7B} x{v r7B}
This is obviously wrong, since the occurrence of `f` to the right of the
arrow refers to the `x` binder (they share a Unique). However, it is
rather confusing when Lint reports
  Mismatch in type between binder and occurrence
  Var: x{v rB7}
since it is printing the binder, rather than the occurrence.
It is rather easy to read this as claiming there is something wrong with
the `x` occurrence!

We change the report to explicitly print both the binder and the
occurrence variables.

- - - - -
7b2c827b by Simon Peyton Jones at 2020-03-10T16:31:15Z
Comments only

Clarify code added in #17852 and MR !2724

- - - - -
3300eeac by Krzysztof Gogolewski at 2020-03-10T16:31:54Z
Misc cleanup

- Remove Note [Existentials in shift_con_pat].
  The function shift_con_pat has been removed 15 years ago in 23f40f0e9be6d4.
- Remove kcLookupTcTyCon - it's the same as tcLookupTcTyCon
- Remove ASSERT in tyConAppArgN. It's already done by getNth,
  and it's the only reason getNth exists.
- Remove unused function nextRole

- - - - -
abf5736b by Krzysztof Gogolewski at 2020-03-10T17:05:01Z
Typos in comments [skip ci]

- - - - -
bb586f89 by Ben Gamari at 2020-03-11T04:14:59Z
rts: Prefer darwin-specific getCurrentThreadCPUTime

macOS Catalina now supports a non-POSIX-compliant version of clock_gettime
which cannot use the clock_gettime codepath.

Fixes #17906.

- - - - -
20800b9a by Sylvain Henry at 2020-03-11T12:17:19Z
Split GHC.Iface.Utils module

* GHC.Iface.Recomp: recompilation avoidance stuff
* GHC.Iface.Make: mkIface*

Moved `writeIfaceFile` into GHC.Iface.Load alongside `readIface` and
renamed it `writeIface` for consistency.

- - - - -
1daa2029 by Greg Steuck at 2020-03-11T12:17:56Z
Fixed a minor typo in codegen.rst
- - - - -
0bc23338 by Ryan Scott at 2020-03-11T12:18:32Z
Re-quantify when generalising over rewrite rule types

Previously, `tcRules` would check for naughty quantification
candidates (see `Note [Naughty quantification candidates]` in
`TcMType`) when generalising over the type of a rewrite rule. This
caused sensible-looking rewrite rules (like those in #17710) to be
rejected. A more permissing (and easier-to-implement) approach is to
do what is described in `Note [Generalising in tcTyFamInstEqnGuts]`
in `TcTyClsDecls`: just re-quantify all the type variable binders,
regardless of the order in which the user specified them. After all,
the notion of type variable specificity has no real meaning in
rewrite rules, since one cannot "visibly apply" a rewrite rule.
I have written up this wisdom in
`Note [Re-quantify type variables in rules]` in `TcRules`.

As a result of this patch, compiling the `ExplicitForAllRules1` test
case now generates one fewer warning than it used to. As far as I can
tell, this is benign, since the thing that the disappearing warning
talked about was also mentioned in an entirely separate warning.

Fixes #17710.

- - - - -
336eac7e by Ben Gamari at 2020-03-11T12:19:08Z
testsuite: Mark ghci056 and ghcilink004 as fragile in unreg

As noted in #17018.

Also fix fragile declaration of T13786, which only runs in the normal
way.

- - - - -
c61b9b02 by Simon Peyton Jones at 2020-03-11T12:19:44Z
Deepen call stack for isIn

I see quite a few warnings like:

  WARNING: file compiler/utils/Util.hs, line 593
    Over-long elem in unionLists

But the call stack is uninformative.   Better to add HasDebugCallStack
to isIn.  Ditto isn'tIn.

- - - - -
3aa9b35f by Ömer Sinan Ağacan at 2020-03-11T12:20:27Z
Zero any slop after compaction in compacting GC

In copying GC, with the relevant debug flags enabled, we release the old
blocks after a GC, and the block allocator zeroes the space before
releasing a block. This effectively zeros the old heap.

In compacting GC we reuse the blocks and previously we didn't zero the
unused space in a compacting generation after compaction. With this
patch we zero the slop between the free pointer and the end of the block
when we're done with compaction and when switching to a new block
(because the current block doesn't have enough space for the next object
we're shifting).

- - - - -
8e6febce by Sylvain Henry at 2020-03-12T00:33:37Z
Refactor GHC.Driver.Session (Ways and Flags)

* extract flags and ways into their own modules (with some renaming)

* remove one SOURCE import of GHC.Driver.Session from GHC.Driver.Phases

* when GHC uses dynamic linking (WayDyn), `interpWays` was only
  reporting WayDyn even if the host was profiled (WayProf).  Now it
  returns both as expected (might fix #16803).

* `mkBuildTag :: [Way] -> String` wasn't reporting a canonical tag for
  differently ordered lists. Now we sort and nub the list to fix this.

- - - - -
bc41e471 by Sylvain Henry at 2020-03-12T00:33:37Z
Refactor interpreterDynamic and interpreterProfiled

* `interpreterDynamic` and `interpreterProfiled` now take `Interp`
  parameters instead of DynFlags

* slight refactoring of `ExternalInterp` so that we can read the iserv
  configuration (which is pure) without reading an MVar.

- - - - -
a6989971 by Sylvain Henry at 2020-03-12T00:33:37Z
Use a Set to represent Ways

Should make `member` queries faster and avoid messing up with missing
`nubSort`.

Metric Increase:
    hie002

- - - - -
cb93a1a4 by Ryan Scott at 2020-03-12T00:34:14Z
Make DeriveFunctor-generated code require fewer beta reductions

Issue #17880 demonstrates that `DeriveFunctor`-generated code is
surprisingly fragile when rank-_n_ types are involved. The culprit is
that `$fmap` (the algorithm used to generate `fmap` implementations)
was too keen on applying arguments with rank-_n_ types to lambdas,
which fail to typecheck more often than not.

In this patch, I change `$fmap` (both the specification and the
implementation) to produce code that avoids creating as many lambdas,
avoiding problems when rank-_n_ field types arise.
See the comments titled "Functor instances" in `TcGenFunctor` for a
more detailed description. Not only does this fix #17880, but it also
ensures that the code that `DeriveFunctor` generates will continue
to work after simplified subsumption is implemented (see #17775).

What is truly amazing is that #17880 is actually a regression
(introduced in GHC 7.6.3) caused by commit
49ca2a37bef18aa57235ff1dbbf1cc0434979b1e, the fix #7436. Prior to
that commit, the version of `$fmap` that was used was almost
identical to the one used in this patch! Why did that commit change
`$fmap` then? It was to avoid severe performance issues that would
arise for recursive `fmap` implementations, such as in the example
below:

```hs
data List a = Nil | Cons a (List a) deriving Functor

-- ===>

instance Functor List where
  fmap f Nil = Nil
  fmap f (Cons x xs) = Cons (f x) (fmap (\y -> f y) xs)
```

The fact that `\y -> f y` was eta expanded caused significant
performance overheads. Commit
49ca2a37bef18aa57235ff1dbbf1cc0434979b1e fixed this performance
issue, but it went too far. As a result, this patch partially
reverts 49ca2a37bef18aa57235ff1dbbf1cc0434979b1e.

To ensure that the performance issues pre-#7436 do not resurface,
I have taken some precautionary measures:

* I have added a special case to `$fmap` for situations where the
  last type variable in an application of some type occurs directly.
  If this special case fires, we avoid creating a lambda expression.
  This ensures that we generate
  `fmap f (Cons x xs) = Cons (f x) (fmap f xs)` in the derived
  `Functor List` instance above. For more details, see
  `Note [Avoid unnecessary eta expansion in derived fmap implementations]`
  in `TcGenFunctor`.
* I have added a `T7436b` test case to ensure that the performance
  of this derived `Functor List`-style code does not regress.

When implementing this, I discovered that `$replace`, the algorithm
which generates implementations of `(<$)`, has a special case that is
very similar to the `$fmap` special case described above. `$replace`
marked this special case with a custom `Replacer` data type, which
was a bit overkill. In order to use the same machinery for both
`Functor` methods, I ripped out `Replacer` and instead implemented
a simple way to detect the special case. See the updated commentary
in `Note [Deriving <$]` for more details.

- - - - -
1f9db3e7 by Kirill Elagin at 2020-03-12T13:45:51Z
pretty-printer: Properly parenthesise LastStmt

After ApplicatveDo strips the last `return` during renaming, the pretty
printer has to restore it. However, if the return was followed by `$`,
the dollar was stripped too and not restored.

For example, the last stamement in:

```
  foo = do
    x <- ...
    ...
    return $ f x
```

would be printed as:

```
    return f x
```

This commit preserved the dolar, so it becomes:

```
    return $ f x
```

- - - - -
5cb93af7 by Kirill Elagin at 2020-03-12T13:45:51Z
pretty-printer: Do not print ApplicativeDo join

* Do not print `join` in ApplictiveStmt, unless ppr-debug
* Print parens around multiple parallel binds

When ApplicativeDo is enabled, the renamer analyses the statements of a
`do` block and in certain cases marks them as needing to be rewritten
using `join`.

For example, if you have:

```
foo = do
  a <- e1
  b <- e2
  doSomething a b
```

it will be desugared into:

```
foo = join (doSomething <$> e1 <*> e2)
```

After renaming but before desugaring the expression is stored
essentially as:

```
foo = do
  [will need join] (a <- e1 | b <- e2)
  [no return] doSomething a b
```

Before this change, the pretty printer would print a call to `join`,
even though it is not needed at this stage at all. The expression will be
actually rewritten into one using join only at desugaring, at which
point a literal call to join will be inserted.

- - - - -
3a259092 by Simon Peyton Jones at 2020-03-12T13:46:29Z
Expose compulsory unfoldings always

The unsafeCoerce# patch requires that unsafeCoerce# has
a compulsory unfolding that is always available.  So we have
to be careful to expose compulsory unfoldings unconditionally
and consistently.

We didn't get this quite right: #17871.  This patch fixes
it.  No real surprises here.

See Note [Always expose compulsory unfoldings] in GHC.Iface.Tidy

- - - - -
6a65b8c2 by Alp Mestanogullari at 2020-03-13T06:29:20Z
hadrian: improve dependency tracking for the check-* programs

The code in Rules.Register responsible for finding all the build artifacts
that Cabal installs when registering a library (static/shared libs, .hi files,
...) was looking in the wrong place. This patch fixes that logic and makes sure
we gather all those artifacts in a list to declare that the rule for a given
`.conf` file, our proxy for "Hadrian, please install this package in the package
db for this stage", also produces those artifacts under the said package
database.

We also were completely missing some logic to declare that the check-* programs
have dependencies besides their source code, at least when testing an in-tree
compiler.

Finally, this patch also removes redundant packages from 'testsuitePackages',
since they should already be covered by the stage<N>Packages lists from
Settings.Default.

With this patch, after a complete build and freezing stage 1, a change to
`compiler/parser/Parser.y` results in rebuilding the ghc lib, reinstalling it,
and rebuilding the few programs that depend on it, _including_ `check-ppr` and
`check-api-annotations` (therefore fixing #17273).

- - - - -
44fad4a9 by Sylvain Henry at 2020-03-13T06:30:22Z
Rename isDllName

I wanted to fix the dangling comment in `isDllName` ("This is the cause
of #", #8696 is already mentioned earlier). I took the opportunity to
change the function name to better reflect what it does.

- - - - -
2f292db8 by Paavo at 2020-03-13T06:31:03Z
Update documentation for closureSize

- - - - -
f124ff0d by Ben Gamari at 2020-03-13T06:31:40Z
gitlab-ci: Rework triggering of release builds

Use a push option instead of tagging.

- - - - -
7f25557a by Ben Gamari at 2020-03-13T14:38:09Z
gitlab-ci: Distinguish integer-simple test envs

Previously two integer-simple jobs declared the same test environment. One (the nightly job) was built in the perf way, the other in the validate way. Consequently they had appreciably different performance characteristics, causing in the nightly job to spuriously fail with performance changes.
- - - - -
c12a2ec5 by Simon Peyton Jones at 2020-03-14T09:25:30Z
Fix Lint

Ticket #17590 pointed out a bug in the way the linter dealt with
type lets, exposed by the new uniqAway story.

The fix is described in Note [Linting type lets]. I ended up
putting the in-scope Ids in a different env field, le_ids,
rather than (as before) sneaking them into the TCvSubst.

Surprisingly tiresome, but done.

Metric Decrease:
    hie002

- - - - -
b989845e by Sylvain Henry at 2020-03-14T09:26:11Z
Hadrian: fix absolute buildroot support (#17822)

Shake's "**" wildcard doesn't match absolute root. We must use "//" instead.

- - - - -
4f117135 by Sylvain Henry at 2020-03-14T09:26:49Z
Make: refactor GMP rules

Document and use simpler rules for the ghc-gmp.h header.

- - - - -
7432b327 by Sylvain Henry at 2020-03-14T09:27:28Z
Use correct option name (-opti) (fix #17314)

s/pgmo/opti

- - - - -
8f7dd571 by Judah Jacobson at 2020-03-14T09:28:07Z
Allow overriding LD_STAGE0 and AR_STAGE0 in the configure script.

Previously it was possible to override the stage0 C compiler via `CC_STAGE0`,
but you couldn't override `ld` or `ar` in stage0.  This change allows overriding them
by setting `LD_STAGE0` or `AR_STAGE0`, respectively.

Our team uses this feature internally to take more control of our GHC build
and make it run more hermetically.

- - - - -
7c3e39a9 by Judah Jacobson at 2020-03-14T09:28:07Z
Use AC_ARG_VAR for LD_STAGE0 and AR_STAGE0.

- - - - -
20d4d676 by Ben Gamari at 2020-03-14T09:28:43Z
nonmoving: Don't traverse filled segment list in pause

The non-moving collector would previously walk the entire filled segment
list during the preparatory pause. However, this is far more work than
is strictly necessary. We can rather get away with merely collecting the
allocators' filled segment list heads and process the lists themselves
during the concurrent phase. This can significantly reduce the maximum
gen1 GC pause time in programs with high rates of long-lived allocations.

- - - - -
fdfa2d01 by Ben Gamari at 2020-03-14T09:29:18Z
nonmoving: Remove redundant bitmap clearing

nonmovingSweep already clears the bitmap in the sweep loop. There is no
reason to do so a second time.

- - - - -
2f8c7767 by Simon Peyton Jones at 2020-03-14T09:29:55Z
Simple refactor of cheapEqExpr

No change in functionality.  Just seems tidier (and signficantly more
efficient) to deal with ticks directly than to call stripTicksTopE.

- - - - -
88f7a762 by Simon Peyton Jones at 2020-03-14T09:29:55Z
Improve CSE.combineAlts

This patch improves the way that CSE combines identical
alternatives.  See #17901.

I'm still not happy about the duplication between CSE.combineAlts
and GHC.Core.Utils.combineIdenticalAlts; see the Notes with those
functions.  But this patch is a step forward.

Metric Decrease:
    T12425
    T5642

- - - - -
8b95ddd3 by Ben Gamari at 2020-03-14T09:30:31Z
gitlab-ci: Add integer-simple release build for Windows

Closes #16144.

- - - - -
e3c374cc by Simon Peyton Jones at 2020-03-14T09:31:07Z
Wrap an implication around class-sig kind errors

Ticket #17841 showed that we can get a kind error
in a class signature, but lack an enclosing implication
that binds its skolems.

This patch

* Adds the wrapping implication: the new call to
  checkTvConstraints in tcClassDecl1

* Simplifies the API to checkTvConstraints, which
  was not otherwise called at all.

* Simplifies TcErrors.report_unsolved by *not*
  initialising the TidyEnv from the typechecker lexical
  envt.  It's enough to do so from the free vars of the
  unsolved constraints; and we get silly renamings if
  we add variables twice: once from the lexical scope
  and once from the implication constraint.

- - - - -
73133a3b by Simon Peyton Jones at 2020-03-14T09:31:07Z
Refactoring in TcSMonad

This patch is just refactoring: no change in
behaviour.

I removed the rather complicated
    checkConstraintsTcS
    checkTvConstraintsTcS

in favour of simpler functions
    emitImplicationTcS
    emitTvImplicationTcS
    pushLevelNoWorkList

The last of these is a little strange, but overall
it's much better I think.

- - - - -
93c88c26 by Ben Gamari at 2020-03-14T09:31:42Z
base: Make `open` calls interruptible

As noted in #17912, `open` system calls were `safe` rather than
`interruptible`. Consequently, the program could not be interrupted with
SIGINT if stuck in a slow open operation. Fix this by marking
`c_safe_open` as interruptible.

- - - - -
bee4cdad by Vladislav Zavialov at 2020-03-14T09:32:18Z
Remove second tcLookupTcTyCon in tcDataDefn

Before this patch, tcDataDefn used to call tcLookupTcTyCon twice in a row:
	1. in bindTyClTyVars itself
	2. in the continuation passed to it

Now bindTyClTyVars passes the TcTyCon to the continuation, making
the second lookup unnecessary.

- - - - -
3f116d35 by Cale Gibbard at 2020-03-14T23:34:42Z
Enable stage1 build of haddock

The submodule has already been bumped to contain the fix.

- - - - -
49e9d739 by Ömer Sinan Ağacan at 2020-03-14T23:35:24Z
rts: Fix printClosure when printing fwd ptrs

- - - - -
1de3ab4a by Krzysztof Gogolewski at 2020-03-14T23:36:04Z
Remove unused field var_inline (#17915)

- - - - -
d30aeb4b by Krzysztof Gogolewski at 2020-03-15T07:57:41Z
Document restriction on SCC pragma syntax

Currently, the names of cost centres must be quoted or
be lowercase identifiers.

Fixes #17916.

- - - - -
b4774598 by Brian Foley at 2020-03-15T07:58:18Z
Remove some dead code

>From the notes.ghc.drop list found using weeder in #17713

- - - - -
dd6ffe6b by Viktor Dukhovni at 2020-03-15T07:58:55Z
Note platform-specific Foreign.C.Types in context

Also fix the markup in the general note at the top of the module.  Haddock
(usability trade-off), does not support multi-line emphasised text.

- - - - -
2e82465f by Sylvain Henry at 2020-03-15T14:57:10Z
Refactor CmmToAsm (disentangle DynFlags)

This patch disentangles a bit more DynFlags from the native code
generator (CmmToAsm).

In more details:

- add a new NCGConfig datatype in GHC.CmmToAsm.Config which contains the
  configuration of a native code generation session
- explicitly pass NCGConfig/Platform arguments when necessary
- as a consequence `sdocWithPlatform` is gone and there are only a few
  `sdocWithDynFlags` left
- remove the use of `unsafeGlobalDynFlags` from GHC.CmmToAsm.CFG
- remove `sdocDebugLevel` (now we pass the debug level via NCGConfig)

There are still some places where DynFlags is used, especially because
of pretty-printing (CLabel), because of Cmm helpers (such as
`cmmExprType`) and because of `Outputable` instance for the
instructions. These are left for future refactoring as this patch is
already big.

- - - - -
c35c545d by Judah Jacobson at 2020-03-15T14:57:48Z
Add a -no-haddock flag.

This flag undoes the effect of a previous "-haddock" flag.  Having both flags makes it easier
for build systems to enable Haddock parsing in a set of global flags, but then disable it locally for
specific targets (e.g., third-party packages whose comments don't pass the validation in the latest GHC).

I added the flag to expected-undocumented-flags.txt since `-haddock` was alreadyin that list.

- - - - -
cfcc3c9a by Ömer Sinan Ağacan at 2020-03-15T14:58:27Z
Fix global_link of TSOs for threads reachable via dead weaks

Fixes #17785

Here's how the problem occurs:

- In generation 0 we have a TSO that is finished (i.e. it has no more
  work to do or it is killed).

- The TSO only becomes reachable after collectDeadWeakPtrs().

- After collectDeadWeakPtrs() we switch to WeakDone phase where we don't
  move TSOs to different lists anymore (like the next gen's thread list
  or the resurrected_threads list).

- So the TSO will never be moved to a generation's thread list, but it
  will be promoted to generation 1.

- Generation 1 collected via mark-compact, and because the TSO is
  reachable it is marked, and its `global_link` field, which is bogus at
  this point (because the TSO is not in a list), will be threaded.

- Chaos ensues.

In other words, when these conditions hold:

- A TSO is reachable only after collectDeadWeakPtrs()
- It's finished (what_next is ThreadComplete or ThreadKilled)
- It's retained by mark-compact collector (moving collector doesn't
  evacuate the global_list field)

We end up doing random mutations on the heap because the TSO's
global_list field is not valid, but it still looks like a heap pointer
so we thread it during compacting GC.

The fix is simple: when we traverse old_threads lists to resurrect
unreachable threads the threads that won't be resurrected currently
stays on the old_threads lists. Those threads will never be visited
again by MarkWeak so we now reset the global_list fields. This way
compacting GC does not thread pointers to nowhere.

Testing
-------

The reproducer in #17785 is quite large and hard to build, because of
the dependencies, so I'm not adding a regression test.

In my testing the reproducer would take a less than 5 seconds to run,
and once in every ~5 runs would fail with a segfault or an assertion
error. In other cases it also fails with a test failure. Because the
tests never fail with the bug fix, assuming the code is correct, this
also means that this bug can sometimes lead to incorrect runtime
results.

After the fix I was able to run the reproducer repeatedly for about an
hour, with no runtime crashes or test failures.

To run the reproducer clone the git repo:

    $ git clone https://github.com/osa1/streamly --branch ghc-segfault

Then clone primitive and atomic-primops from their git repos and point
to the clones in cabal.project.local. The project should then be
buildable using GHC HEAD. Run the executable `properties` with `+RTS -c
-DZ`.

In addition to the reproducer above I run the test suite using:

    $ make slowtest EXTRA_HC_OPTS="-debug -with-rtsopts=-DS \
        -with-rtsopts=-c +RTS -c -RTS" SKIPWAY='nonmoving nonmoving_thr'

This enables compacting GC always in both GHC when building the test
programs and when running the test programs, and also enables sanity
checking when running the test programs. These set of flags are not
compatible for all tests so there are some failures, but I got the same
set of failures with this patch compared to GHC HEAD.

- - - - -
818b3c38 by Lysxia at 2020-03-17T03:52:42Z
base: add strict IO functions: readFile', getContents', hGetContents'

- - - - -
18a346a4 by Sylvain Henry at 2020-03-17T03:53:24Z
Modules: Core (#13009)

Update submodule: haddock

- - - - -
92327e3a by Ömer Sinan Ağacan at 2020-03-17T03:54:04Z
Update sanity checking for TSOs:

- Remove an invalid assumption about GC checking what_next field. The GC
  doesn't care about what_next at all, if a TSO is reachable then all
  its pointers are followed (other than global_tso, which is only
  followed by compacting GC).

- Remove checkSTACK in checkTSO: TSO stacks will be visited in
  checkHeapChain, or checkLargeObjects etc.

- Add an assertion in checkTSO to check that the global_link field is
  sane.

- Did some refactor to remove forward decls in checkGlobalTSOList and
  added braces around single-statement if statements.

- - - - -
5ac04eed by Sebastian Graf at 2020-03-17T10:05:58Z
Preserve precise exceptions in strictness analysis

The "IO hack" (which is a fallback to preserve precise exceptions
semantics and thus *soundness*, rather than some smart thing that
increases *precision*) always was quite hard to understand. That led to
a misguided effort to simplify it (!1829), because the Note wasn't
particularly clear about what kinds of side-effects it cares about.

The implementation seemed to care about preserving precise exception
semantics, but failed to deliver for the central case of `raiseIO#`
(#17676), which is in stark contrast to one of the motivating examples
in the Note (the one about `exitWith ExitSuccess`).

This patch rewords the Note to apply to IO actions throwing precise
exceptions, rather than all side-effecting IO actions (such as write
effects) in general. Also it makes this clear in the implementation by
extracting the rather opaque `io_hack_reqd` into
`CoreUtils.exprMightThrowPreciseException`.

In fact, that alone wasn't enough to fix #17676. It actually turned out
to be a duplicate of #13380, for which we had a fix in 7b087aeb, making
`catchIO#` have `topDiv` from `botDiv`. But that was reverted on the
grounds of regressing dead code elimination too much. In this patch
we introduce `exnDiv` for `raiseIO#`, the `defaultDmd` of which acts
like `topDiv`s (which was the key point which fixed #13380), but
otherwise acts like `botDiv` in terms of dead code elimination.

Fixes #13380 and #17676.

- - - - -
80f00345 by Sebastian Graf at 2020-03-17T10:05:58Z
Add ConOrDiv to Divergence and see where it gets us

- - - - -
9760f64c by Sebastian Graf at 2020-03-17T10:05:58Z
Actually use conDiv

- - - - -
ff735c48 by Sebastian Graf at 2020-03-17T19:04:23Z
Attempt to make ensureArgs do the right thing

- - - - -


30 changed files:

- .gitlab-ci.yml
- compiler/GHC.hs
- compiler/GHC/ByteCode/Asm.hs
- compiler/GHC/ByteCode/InfoTable.hs
- compiler/GHC/ByteCode/Instr.hs
- compiler/GHC/ByteCode/Types.hs
- compiler/GHC/Cmm/CLabel.hs
- compiler/GHC/Cmm/Dataflow/Block.hs
- compiler/GHC/Cmm/Parser.y
- compiler/GHC/Cmm/Utils.hs
- compiler/GHC/CmmToAsm.hs
- compiler/GHC/CmmToAsm/BlockLayout.hs
- compiler/GHC/CmmToAsm/CFG.hs
- + compiler/GHC/CmmToAsm/Config.hs
- compiler/GHC/CmmToAsm/Dwarf.hs
- compiler/GHC/CmmToAsm/Dwarf/Constants.hs
- compiler/GHC/CmmToAsm/Dwarf/Types.hs
- compiler/GHC/CmmToAsm/Instr.hs
- compiler/GHC/CmmToAsm/Monad.hs
- compiler/GHC/CmmToAsm/PIC.hs
- compiler/GHC/CmmToAsm/PPC/CodeGen.hs
- compiler/GHC/CmmToAsm/PPC/Instr.hs
- compiler/GHC/CmmToAsm/PPC/Ppr.hs
- compiler/GHC/CmmToAsm/PPC/Regs.hs
- compiler/GHC/CmmToAsm/Ppr.hs
- compiler/GHC/CmmToAsm/Reg/Graph.hs
- compiler/GHC/CmmToAsm/Reg/Graph/Stats.hs
- compiler/GHC/CmmToAsm/Reg/Linear.hs
- compiler/GHC/CmmToAsm/Reg/Linear/Base.hs
- compiler/GHC/CmmToAsm/Reg/Linear/FreeRegs.hs


The diff was not included because it is too large.


View it on GitLab: https://gitlab.haskell.org/ghc/ghc/compare/38fa79111a873e5f519bd952e665c7a770d3961c...ff735c48a5f9aeddf9640c0ffb12013a07ad3fec

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/compare/38fa79111a873e5f519bd952e665c7a770d3961c...ff735c48a5f9aeddf9640c0ffb12013a07ad3fec
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/20200317/f4ff1c52/attachment-0001.html>


More information about the ghc-commits mailing list