[Git][ghc/ghc][wip/memory-barriers] 36 commits: gitlab-ci: Linters, don't allow to fail

Ben Gamari gitlab at gitlab.haskell.org
Mon Jun 10 20:07:54 UTC 2019



Ben Gamari pushed to branch wip/memory-barriers at Glasgow Haskell Compiler / GHC


Commits:
07dc79c3 by Matthew Pickering at 2019-06-08T17:34:18Z
gitlab-ci: Linters, don't allow to fail

Ben disabled them in cd85f8a71bb56cff332560e1d571b3406789fb71 but didn't
say how or why they were broken.

- - - - -
fd840b64 by Matthew Pickering at 2019-06-08T17:34:18Z
gitlab-ci: Don't run two submodule checking jobs on Marge jobs

- - - - -
310d0c4c by Matthew Pickering at 2019-06-08T17:34:18Z
Fix two lint failures in rts/linker/MachO.c

- - - - -
fe965316 by Ben Gamari at 2019-06-08T17:34:18Z
gitlab-ci: Use --unshallow when fetching for linters

GitLab creates a shallow clone. However, this means that we may not have
the base commit of an MR when linting, causing `git merge-base` to fail.
Fix this by passing `--unshallow` to `git fetch`, ensuring that we have
the entire history.

- - - - -
f58234ea by Ben Gamari at 2019-06-08T17:34:18Z
gitlab-ci: Fix submodule linter

The job script didn't even try to compute the base commit to lint with
respect to.

- - - - -
c392f987 by Ben Gamari at 2019-06-08T17:34:18Z
gitlab-ci: A few clarifying comments

- - - - -
709290b0 by Matthew Pickering at 2019-06-08T17:38:15Z
Remove trailing whitespace

[skip ci]

This should really be caught by the linters! (#16711)

- - - - -
b2f106f5 by Ben Gamari at 2019-06-08T18:02:02Z
gitlab-ci: Disable shallow clones

Previously we were passing `--unshallow` to `git fetch` in the linting
rules to ensure that the base commit which we were linting with respect
to was available. However, this breaks due to GitLab's re-use of
working directories since `git fetch --unshallow` fails on a repository
which is not currently shallow.

Given that `git fetch --unshallow` circumvents the efficiencies provided
by shallow clones anyways, let's just disable them entirely.

There is no documented way to do disable shallow clones but on checking
the GitLab implementation it seems that setting `GIT_DEPTH=0` should do
the trick.

- - - - -
4a72259d by Ben Gamari at 2019-06-08T18:40:55Z
gitlab-ci: Fix submodule linting of commits

There is no notion of a base commit when we aren't checking a merge
request. Just check the HEAD commit.

- - - - -
87540029 by Ben Gamari at 2019-06-08T20:44:55Z
gitlab-ci: Ensure that all commits on a branch are submodule-linted

The previous commit reworked things such that the submodule linter would
only run on the head commit. However, the linter only checks the
submodules which are touched by the commits it is asked to lint.
Consequently it would be possible for a bad submodule to sneak through.

Thankfully, we can use the handy CI_COMMIT_BEFORE_SHA attribute to
find the base commit of the push.

- - - - -
0462b0e0 by Alexandre Baldé at 2019-06-09T15:48:34Z
Explain that 'mappend' and '(<>)' should be the same [skip ci]

- - - - -
970e4802 by Matthew Pickering at 2019-06-09T15:49:09Z
hadrian: Properly partition options in sourceArgs

Previously if you build the `ghc` package then it would has the default
opts and the library opts. This is different behaviour to make where the
library opts are only reserved for things in the `libraries`
subdirectory (I believe)

Fixes #16716

- - - - -
a018c3a8 by Ben Gamari at 2019-06-09T15:49:44Z
testsuite: Suppress ticks in T4918 output

As noted in #16741, this test otherwise breaks when `base` is compiled
with `-g`.

- - - - -
f7370333 by chessai at 2019-06-09T22:41:02Z
Introduce log1p and expm1 primops

Previously log and exp were primitives yet log1p and expm1 were FFI
calls. Fix this non-uniformity.

- - - - -
41bf4045 by Ben Gamari at 2019-06-09T22:41:38Z
testsuite: Add test for #16514

- - - - -
b9fe91fc by Simon Jakobi at 2019-06-09T22:42:21Z
Small refactorings in ExtractDocs

- - - - -
9d238791 by Kevin Buhr at 2019-06-09T22:42:57Z
Handle trailing path separator in package DB names (#16360)

Package DB directories with trailing separator (provided via
GHC_PACKAGE_PATH or via -package-db) resulted in incorrect calculation of
${pkgroot} substitution variable.  Keep the trailing separator while
resolving as directory or file, but remove it before dropping the last
path component with takeDirectory.

Closes #16360.

- - - - -
a22e51ea by Richard Eisenberg at 2019-06-09T22:43:38Z
Fix #16517 by bumping the TcLevel for method sigs

There were actually two bugs fixed here:

1. candidateQTyVarsOfType needs to be careful that it does not
   try to zap metavariables from an outer scope as "naughty"
   quantification candidates. This commit adds a simple check
   to avoid doing so.

2. We weren't bumping the TcLevel in kcHsKindSig, which was used
   only for class method sigs. This mistake led to the acceptance
   of

     class C a where
       meth :: forall k. Proxy (a :: k) -> ()

   Note that k is *locally* quantified. This patch fixes the
   problem by using tcClassSigType, which correctly bumps the
   level. It's a bit inefficient because tcClassSigType does other
   work, too, but it would be tedious to repeat much of the code
   there with only a few changes. This version works well and is
   simple.

And, while updating comments, etc., I noticed that tcRnType was
missing a pushTcLevel, leading to #16767, which this patch also
fixes, by bumping the level. In the refactoring here, I also
use solveEqualities. This initially failed ghci/scripts/T15415,
but that was fixed by teaching solveEqualities to respect
-XPartialTypeSignatures.

This patch also cleans up some Notes around error generation that
came up in conversation.

Test case: typecheck/should_fail/T16517, ghci/scripts/T16767

- - - - -
10452959 by Roland Senn at 2019-06-09T22:44:18Z
Add disable/enable commands to ghci debugger #2215

This patch adds two new commands `:enable` and `:disable` to the GHCi debugger.
Opposite to `:set stop <n> :continue` a breakpoint disabled with `:disable` will
not loose its previously set stop command.
A new field breakEnabled is added to the BreakLocation data structure to
track the enable/disable state. When a breakpoint is disabled with a `:disable`
command, the following happens:

The corresponding BreakLocation data element is searched dictionary of the
`breaks` field of the GHCiStateMonad. If the break point is found and not
already in the disabled state, the breakpoint is removed from bytecode.
The BreakLocation data structure is kept in the breaks list and the new
breakEnabled field is set to false.

The `:enable` command works similar.

The breaks field in the GHCiStateMonad was changed from an association list
to int `IntMap`.

- - - - -
13572480 by Ben Gamari at 2019-06-09T22:44:54Z
rts: Separate population of eventTypes from initial event generation

Previously these two orthogonal concerns were both implemented in
postHeaderEvents which made it difficult to send header events after RTS
initialization.

- - - - -
ed20412a by nineonine at 2019-06-09T22:45:31Z
Do not report error if Name in pragma is unbound

- - - - -
8a48a8a4 by Ben Gamari at 2019-06-09T22:46:08Z
testsuite: Add test for #16509

- - - - -
69c58f8a by David Eichmann at 2019-06-09T22:46:46Z
Hadrian: need CPP preprocessor dependencies #16660

Use the new -include-cpp-deps ghc option (#16521)
when generating .dependencies files in hadrian.
This is version gated as -include-cpp-deps is a
relatively new option.

- - - - -
1c7bb03d by Richard Eisenberg at 2019-06-09T22:47:24Z
Comments only: document tcdDataCusk better.

- - - - -
5023adce by John Ericson at 2019-06-09T22:47:59Z
Remove CPP ensuring word size is 32 or 64 bits around Addr# <-> int# primops

It shouldn't be needed these days, and those primops are "highly
deprecated" anyways.

This fits with my plans because it removes one bit of target-dependence
of the builtin primops, and this is the hardest part of GHC to make
multi-target.

CC @carter

- - - - -
8e60e3f0 by Daniel Gröber at 2019-06-09T22:48:38Z
rts: Fix RetainerProfile early return with TREC_CHUNK

When pop() returns with `*c == NULL` retainerProfile will immediately
return. All other code paths is pop() continue with the next stackElement
when this happens so it seems weird to me that TREC_CHUNK we would suddenly
abort everything even though the stack might still have elements left to
process.

- - - - -
1a3420ca by Ben Gamari at 2019-06-10T11:59:41Z
base: Mark CPUTime001 as fragile

As noted in #16224, CPUTime001 has been quite problematic, reporting
non-monotonic timestamps in CI. Unfortunately I've been unable to
reproduce this locally.

- - - - -
9bc10993 by Vladislav Zavialov at 2019-06-10T12:00:16Z
Print role annotations in TemplateHaskell brackets (#16718)

- - - - -
9887cc31 by Travis Whitaker at 2019-06-10T20:07:45Z
Correct closure observation, construction, and mutation on weak memory machines.

Here the following changes are introduced:
    - A read barrier machine op is added to Cmm.
    - The order in which a closure's fields are read and written is changed.
    - Memory barriers are added to RTS code to ensure correctness on
      out-or-order machines with weak memory ordering.

Cmm has a new CallishMachOp called MO_ReadBarrier. On weak memory machines, this
is lowered to an instruction that ensures memory reads that occur after said
instruction in program order are not performed before reads coming before said
instruction in program order. On machines with strong memory ordering properties
(e.g. X86, SPARC in TSO mode) no such instruction is necessary, so
MO_ReadBarrier is simply erased. However, such an instruction is necessary on
weakly ordered machines, e.g. ARM and PowerPC.

Weam memory ordering has consequences for how closures are observed and mutated.
For example, consider a closure that needs to be updated to an indirection. In
order for the indirection to be safe for concurrent observers to enter, said
observers must read the indirection's info table before they read the
indirectee. Furthermore, the entering observer makes assumptions about the
closure based on its info table contents, e.g. an INFO_TYPE of IND imples the
closure has an indirectee pointer that is safe to follow.

When a closure is updated with an indirection, both its info table and its
indirectee must be written. With weak memory ordering, these two writes can be
arbitrarily reordered, and perhaps even interleaved with other threads' reads
and writes (in the absence of memory barrier instructions). Consider this
example of a bad reordering:

- An updater writes to a closure's info table (INFO_TYPE is now IND).
- A concurrent observer branches upon reading the closure's INFO_TYPE as IND.
- A concurrent observer reads the closure's indirectee and enters it. (!!!)
- An updater writes the closure's indirectee.

Here the update to the indirectee comes too late and the concurrent observer has
jumped off into the abyss. Speculative execution can also cause us issues,
consider:

- An observer is about to case on a value in closure's info table.
- The observer speculatively reads one or more of closure's fields.
- An updater writes to closure's info table.
- The observer takes a branch based on the new info table value, but with the
  old closure fields!
- The updater writes to the closure's other fields, but its too late.

Because of these effects, reads and writes to a closure's info table must be
ordered carefully with respect to reads and writes to the closure's other
fields, and memory barriers must be placed to ensure that reads and writes occur
in program order. Specifically, updates to a closure must follow the following
pattern:

- Update the closure's (non-info table) fields.
- Write barrier.
- Update the closure's info table.

Observing a closure's fields must follow the following pattern:

- Read the closure's info pointer.
- Read barrier.
- Read the closure's (non-info table) fields.

This patch updates RTS code to obey this pattern. This should fix long-standing
SMP bugs on ARM (specifically newer aarch64 microarchitectures supporting
out-of-order execution) and PowerPC. This fixesd issue #15449.

- - - - -
aee9ae9e by Ben Gamari at 2019-06-10T20:07:45Z
rts: Fix memory barriers

This reverts and fixes some of the barriers introduced in the previous
patch. In particular, we only need barriers on closures which are
visible to other cores. This means we can exclude barriers on
newly-allocated closures.

However, when we make a closure visible to other cores (e.g. by
introducing a pointer to it into another possibly-visible closure)
then we must first place a write barrier to ensure that other cores
cannot see a partially constructed closure.

- - - - -
73968f1f by Ben Gamari at 2019-06-10T20:07:45Z
More comments

- - - - -
12e5b6a1 by Ben Gamari at 2019-06-10T20:07:45Z
Add missing memory barrier

- - - - -
61fab614 by Ben Gamari at 2019-06-10T20:07:45Z
Fix weaks

- - - - -
360685c9 by Ben Gamari at 2019-06-10T20:07:45Z
Threads: Shuffle barrier

It seems clearer if it's closer to its use site

- - - - -
24d4f451 by Ben Gamari at 2019-06-10T20:07:45Z
Evac: Drop redundant barrier in serial path

- - - - -
e5821bbb by Ben Gamari at 2019-06-10T20:07:45Z
Fix revertCAFs

- - - - -


30 changed files:

- .gitlab-ci.yml
- compiler/cmm/CmmMachOp.hs
- compiler/cmm/CmmParse.y
- compiler/cmm/MkGraph.hs
- compiler/cmm/PprC.hs
- compiler/codeGen/StgCmmBind.hs
- compiler/codeGen/StgCmmForeign.hs
- compiler/codeGen/StgCmmPrim.hs
- compiler/deSugar/ExtractDocs.hs
- compiler/ghci/LinkerTypes.hs
- compiler/hsSyn/HsDecls.hs
- compiler/llvmGen/LlvmCodeGen/CodeGen.hs
- compiler/main/HscTypes.hs
- compiler/main/Packages.hs
- compiler/nativeGen/PPC/CodeGen.hs
- compiler/nativeGen/SPARC/CodeGen.hs
- compiler/nativeGen/X86/CodeGen.hs
- compiler/prelude/primops.txt.pp
- compiler/rename/RnEnv.hs
- compiler/specialise/Specialise.hs
- compiler/stgSyn/StgSyn.hs
- compiler/typecheck/TcCanonical.hs
- compiler/typecheck/TcErrors.hs
- compiler/typecheck/TcHsType.hs
- compiler/typecheck/TcMType.hs
- compiler/typecheck/TcRnDriver.hs
- compiler/typecheck/TcRnTypes.hs
- compiler/typecheck/TcSimplify.hs
- compiler/typecheck/TcTyClsDecls.hs
- compiler/typecheck/TcType.hs


The diff was not included because it is too large.


View it on GitLab: https://gitlab.haskell.org/ghc/ghc/compare/d2833e62a7c5ac3d24cf158d814927b34922aa73...e5821bbb20c119a2a8e1dfd93666413eaf750f0a

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/compare/d2833e62a7c5ac3d24cf158d814927b34922aa73...e5821bbb20c119a2a8e1dfd93666413eaf750f0a
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/20190610/7fce34ef/attachment-0001.html>


More information about the ghc-commits mailing list