October 2016 - ghc-devs

Long-term storage of abandoned branches
by Joachim Breitner 24 Oct '16

24 Oct '16

Hi, in https://ghc.haskell.org/trac/ghc/ticket/12618#comment:37 Simon raises a question that I was wondering about as well: Where do we want to store feature branches that contain useful work that might be picked up some time later (maybe much later¹)? So far, I have left them as wip/foobar branches. Which works ok, but it clutters up the branch namespace, and “wip” is a lie. Maybe I should move the branch to archive/foobar, to make it clear that this is not something actively worked on? An alternative is having it in Phab only, where it is “more out of the way”, and there is commentary attached to it. Linked from the appropriate ticket, the code is as accessible as a git branch. But are we committed to keeping the Differential Revisions around for years? Also, if the branch contains many small commits, which presumably makes it more useful to whoever revives the project one day, it is easy to recover that from Phab? (I wonder because arc land squashes commits.) Greetings, Joachim ¹ this does happen, see https://ghc.haskell.org/trac/ghc/ticket/1600#comment:52 -- Joachim “nomeata” Breitner mail(a)joachim-breitner.de • https://www.joachim-breitner.de/ XMPP: nomeata(a)joachim-breitner.de • OpenPGP-Key: 0xF0FBF51F Debian Developer: nomeata(a)debian.org

2 2

Dataflow analysis for Cmm
by Michal Terepeta 24 Oct '16

24 Oct '16

Hi, I was looking at cleaning up a bit the situation with dataflow analysis for Cmm. In particular, I was experimenting with rewriting the current `cmm.Hoopl.Dataflow` module: - To only include the functionality to do analysis (since GHC doesn’t seem to use the rewriting part). Benefits: - Code simplification (we could remove a lot of unused code). - Makes it clear what we’re actually using from Hoopl. - To have an interface that works with transfer functions operating on a whole basic block (`Block CmmNode C C`). This means that it would be up to the user of the algorithm to traverse the whole block. Benefits: - Further simplifications. - We could remove `analyzeFwdBlocks` hack, which AFAICS is just a copy&paste of `analyzeFwd` but ignores the middle nodes (probably for efficiency of analyses that only look at the blocks). - More flexible (e.g., the clients could know which block they’re processing; we could consider memoizing some per block information, etc.). What do you think about this? I have a branch that implements the above: https://github.com/michalt/ghc/tree/dataflow2/1 It’s introducing a second parallel implementation (`cmm.Hoopl.Dataflow2` module), so that it's possible to run ./validate while comparing the results of the old implementation with the new one. Second question: how could we merge this? (assuming that people are generally ok with the approach) Some ideas: - Change cmm/Hoopl/Dataflow module itself along with the three analyses that use it in one step. - Introduce the Dataflow2 module first, then switch the analyses, then remove any unused code that still depends on the old Dataflow module, finally remove the old Dataflow module itself. (Personally I'd prefer the second option, but I'm also ok with the first one) I’m happy to export the code to Phab if you prefer - I wasn’t sure what’s the recommended workflow for code that’s not ready for review… Thanks, Michal

4 9

Semantics of MVars and IORefs?
by Iavor Diatchki 24 Oct '16

24 Oct '16

Hello, recently, I and a few colleagues have been wondering about the interaction between IORefs and MVars, and we can't seem to find any explicit documentation stating the behavior, so I was wondering if anyone might know the answer. The question is: is it safe to use IORefs in a multi-threaded program, provided that the uses are within a "critical section" implemented with MVars. Here is a simple program to illustrate the situation: we have two threads, each thread takes a lock, increments a counter, then releases the lock: > import Control.Concurrent > import Data.IORef > > main :: IO () > main = > do lock <- newMVar () > counter <- newIORef 0 > forkIO (thread lock counter) > thread lock counter > > thread :: MVar () -> IORef Integer -> IO a > thread lock counter = > do takeMVar lock > value <- readIORef counter > print value > writeIORef counter (value + 1) > putMVar lock () > thread lock counter The question is if this program has a race condition or not, due to the use of IORefs? More explicitly, the concern is if a write to an IORef in one thread is guaranteed to be seen by a read from the same IORef in another thread, provided that there is proper synchronization between the two. -Iavor

3 2

Aarrgh! Windows build broken again
by Simon Peyton Jones 24 Oct '16

24 Oct '16

On Windows with HEAD I get C:/code/HEAD/inplace/mingw/bin/ld.exe: cannot find -lnuma Sigh. This didn't happen a day or two ago Simon

2 2

RE: [commit: ghc] master: A collection of type-inference refactorings. (3f5673f)
by Simon Peyton Jones 23 Oct '16

23 Oct '16

Friends You'll have seen a brief flurry of commits from me, of which this is the biggest. I've managed to spend some time on the typechecker recently, and this is the result. I'm pleased with the results (simpler code, more predictable behaviour), but although I worked through the changes with Richard, I may have introduced new bugs, so keep an eye out. More on the way! Simon | >--------------------------------------------------------------- | | commit 3f5673f34a2f761423027bf46f64f7499708725f | Author: Simon Peyton Jones <simonpj(a)microsoft.com> | Date: Tue Sep 20 23:31:07 2016 +0100 | | A collection of type-inference refactorings. | | This patch does a raft of useful tidy-ups in the type checker. | I've been meaning to do this for some time, and finally made | time to do it en route to ICFP. | | 1. Modify TcType.ExpType to make a distinct data type, | InferResult for the Infer case, and consequential | refactoring. | | 2. Define a new function TcUnify.fillInferResult, to fill in | an InferResult. It uses TcMType.promoteTcType to promote | the type to the level of the InferResult. | See TcMType Note [Promoting a type] | This refactoring is in preparation for an improvement | to typechecking pattern bindings, coming next. | | I flirted with an elaborate scheme to give better | higher rank inference, but it was just too complicated. | See TcMType Note [Promotion and higher rank types] | | 3. Add to InferResult a new field ir_inst :: Bool to say | whether or not the type used to fill in the | InferResult should be deeply instantiated. See | TcUnify Note [Deep instantiation of InferResult]. | | 4. Add a TcLevel to SkolemTvs. This will be useful generally | | - it's a fast way to see if the type | variable escapes when floating (not used yet) | | - it provides a good consistency check when updating a | unification variable (TcMType.writeMetaTyVarRef, the | level_check_ok check) | | I originally had another reason (related to the flirting | in (2), but I left it in because it seems like a step in | the right direction. | | 5. Reduce and simplify the plethora of uExpType, | tcSubType and related functions in TcUnify. It was | such an opaque mess and it's still not great, but it's | better. | | 6. Simplify the uo_expected field of TypeEqOrigin. Richard | had generatlised it to a ExpType, but it was almost always | a Check type. Now it's back to being a plain TcType which | is much, much easier. | | 7. Improve error messages by refraining from skolemisation when | it's clear that there's an error: see | TcUnify Note [Don't skolemise unnecessarily] | | 8. Type.isPiTy and isForAllTy seem to be missing a coreView check, | so I added it | | 9. Kill off tcs_used_tcvs. Its purpose is to track the | givens used by wanted constraints. For dictionaries etc | we do that via the free vars of the /bindings/ in the | implication constraint ic_binds. But for coercions we | just do update-in-place in the type, rather than | generating a binding. So we need something analogous to | bindings, to track what coercions we have added. | | That was the purpose of tcs_used_tcvs. But it only | worked for a /single/ iteration, whereas we may have | multiple iterations of solving an implication. Look | at (the old) 'setImplicationStatus'. If the constraint | is unsolved, it just drops the used_tvs on the floor. | If it becomes solved next time round, we'll pick up | coercions used in that round, but ignore ones used in | the first round. | | There was an outright bug. Result = (potentialy) bogus | unused-constraint errors. Constructing a case where this | actually happens seems quite trick so I did not do so. | | Solution: expand EvBindsVar to include the (free vars of | the) coercions, so that the coercions are tracked in | essentially the same way as the bindings. | | This turned out to be much simpler. Less code, more | correct. | | 10. Make the ic_binds field in an implication have type | ic_binds :: EvBindsVar | instead of (as previously) | ic_binds :: Maybe EvBindsVar | This is notably simpler, and faster to use -- less | testing of the Maybe. But in the occaional situation | where we don't have anywhere to put the bindings, the | belt-and-braces error check is lost. So I put it back | as an ASSERT in 'setImplicationStatus' (see the use of | 'termEvidenceAllowed') | | All these changes led to quite bit of error message wibbling | | | >--------------------------------------------------------------- | | 3f5673f34a2f761423027bf46f64f7499708725f | compiler/ghci/RtClosureInspect.hs | 2 +- | compiler/typecheck/Inst.hs | 4 +- | compiler/typecheck/TcBinds.hs | 90 +-- | compiler/typecheck/TcErrors.hs | 44 +- | compiler/typecheck/TcEvidence.hs | 33 +- | compiler/typecheck/TcExpr.hs | 24 +- | compiler/typecheck/TcHsSyn.hs | 7 +- | compiler/typecheck/TcHsType.hs | 21 +- | compiler/typecheck/TcInstDcls.hs | 7 +- | compiler/typecheck/TcMType.hs | 324 ++++++--- | compiler/typecheck/TcMatches.hs | 19 +- | compiler/typecheck/TcPat.hs | 22 +- | compiler/typecheck/TcPatSyn.hs | 16 +- | compiler/typecheck/TcPluginM.hs | 16 +- | compiler/typecheck/TcRnDriver.hs | 9 +- | compiler/typecheck/TcRnMonad.hs | 28 +- | compiler/typecheck/TcRnTypes.hs | 29 +- | compiler/typecheck/TcSMonad.hs | 124 ++-- | compiler/typecheck/TcSimplify.hs | 58 +- | compiler/typecheck/TcType.hs | 78 ++- | compiler/typecheck/TcUnify.hs | 731 | +++++++++++++++------ | compiler/typecheck/TcValidity.hs | 2 +- | compiler/types/Type.hs | 2 + | compiler/vectorise/Vectorise/Generic/PData.hs | 2 +- | testsuite/tests/ado/ado004.stderr | 4 +- | .../tests/annotations/should_fail/annfail10.stderr | 12 +- | testsuite/tests/driver/T2182.stderr | 32 +- | testsuite/tests/gadt/gadt-escape1.stderr | 16 +- | testsuite/tests/gadt/gadt13.stderr | 10 +- | testsuite/tests/gadt/gadt7.stderr | 18 +- | .../tests/ghci.debugger/scripts/break012.stdout | 8 +- | .../tests/ghci.debugger/scripts/print022.stdout | 4 +- | testsuite/tests/ghci/scripts/T11524a.stdout | 4 +- | testsuite/tests/ghci/scripts/T2182ghci.stderr | 10 +- | .../tests/indexed-types/should_fail/T12386.hs | 9 + | .../tests/indexed-types/should_fail/T12386.stderr | 7 + | .../tests/indexed-types/should_fail/T5439.stderr | 16 +- | .../tests/indexed-types/should_fail/T7354.stderr | 8 +- | .../tests/parser/should_compile/read014.stderr | 2 +- | testsuite/tests/parser/should_fail/T7848.stderr | 5 +- | .../tests/parser/should_fail/readFail003.stderr | 4 +- | .../partial-sigs/should_compile/T10438.stderr | 14 +- | .../partial-sigs/should_compile/T11192.stderr | 16 +- | .../tests/patsyn/should_compile/T11213.stderr | 2 +- | testsuite/tests/patsyn/should_fail/mono.stderr | 4 +- | testsuite/tests/polykinds/T7438.stderr | 16 +- | testsuite/tests/rebindable/rebindable6.stderr | 12 +- | .../tests/rename/should_compile/T12597.stderr | 2 +- | testsuite/tests/roles/should_compile/T8958.stderr | 5 +- | .../simplCore/should_compile/noinline01.stderr | 4 +- | testsuite/tests/th/T11452.stderr | 2 +- | testsuite/tests/th/T2222.stderr | 2 +- | .../typecheck/should_compile/ExPatFail.stderr | 4 +- | .../should_compile/T12427.stderr} | 0 | .../tests/typecheck/should_compile/T12427a.stderr | 33 + | .../tests/typecheck/should_compile/tc141.stderr | 6 +- | .../tests/typecheck/should_fail/T10495.stderr | 10 +- | .../tests/typecheck/should_fail/T10619.stderr | 4 +- | .../tests/typecheck/should_fail/T12177.stderr | 19 +- | testsuite/tests/typecheck/should_fail/T3102.hs | 6 +- | testsuite/tests/typecheck/should_fail/T3102.stderr | 12 - | testsuite/tests/typecheck/should_fail/T7453.stderr | 50 +- | testsuite/tests/typecheck/should_fail/T7734.stderr | 12 +- | testsuite/tests/typecheck/should_fail/T9109.stderr | 10 +- | testsuite/tests/typecheck/should_fail/T9318.stderr | 12 +- | .../tests/typecheck/should_fail/VtaFail.stderr | 2 +- | testsuite/tests/typecheck/should_fail/all.T | 2 +- | .../tests/typecheck/should_fail/tcfail002.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail004.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail005.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail013.stderr | 2 +- | .../tests/typecheck/should_fail/tcfail014.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail018.stderr | 2 +- | .../tests/typecheck/should_fail/tcfail032.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail099.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail104.stderr | 10 +- | .../tests/typecheck/should_fail/tcfail140.stderr | 4 +- | .../tests/typecheck/should_fail/tcfail181.stderr | 2 +- | .../tests/warnings/should_compile/T12574.stderr | 2 +- | 79 files changed, 1321 insertions(+), 859 deletions(-) | | Diff suppressed because of size. To see it, use: | | git diff-tree --root --patch-with-stat --no-color --find-copies- | harder --ignore-space-at-eol --cc | 3f5673f34a2f761423027bf46f64f7499708725f | _______________________________________________ | ghc-commits mailing list | ghc-commits(a)haskell.org | https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.hask | ell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc- | commits&data=02%7C01%7Csimonpj%40microsoft.com%7Ccf7d693b723d4b7b061b08d3 | f9cda27f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636126634003055994& | sdata=p7fR8mA%2BXBSWTN%2B7pX6B2qs9zYM0CiXPsSI%2BvK21d8Q%3D&reserved=0

2 1

Windows Builder Disabled
by Matthew Pickering 22 Oct '16

22 Oct '16

I have disabled the windows builder as it is consistently failing with the same error message. >From this log for example: https://phabricator.haskell.org/harbormaster/build/14430/ ghc.exe: getMBlocks: VirtualAlloc MEM_COMMIT failed: The paging file is too small for this operation to complete. 764ghc.exe: failed to create OS thread: The paging file is too small for this operation to complete. Do you have any ideas Ben/Tamar? I can't see any recent commits which would have caused this failure. Matt

3 2

RE: [Diffusion] [Build Failed] rGHC6ddba64287fe: Improve TcCanonical.unifyWanted and unifyDerived
by Simon Peyton Jones 21 Oct '16

21 Oct '16

I'm still seeing this OSX failure, below. It seems to be to do with T5611 but I have no way to see what the problem is, and I don't think it's any thing to do with me When I click on "unlimited lines" to attempt to see the error, Firefox hangs, and I have to kill it. It's unsettling getting all these failure messages Simon From: noreply(a)phabricator.haskell.org [mailto:noreply@phabricator.haskell.org] Sent: 21 October 2016 21:32 To: Simon Peyton Jones <simonpj(a)microsoft.com> Subject: [Diffusion] [Build Failed] rGHC6ddba64287fe: Improve TcCanonical.unifyWanted and unifyDerived Harbormaster failed to build B11529: rGHC6ddba64287fe: Improve TcCanonical.unifyWanted and unifyDerived! BRANCHES master USERS simonpj (Author) O11 (Auditor) COMMIT https://phabricator.haskell.org/rGHC6ddba64287fe EMAIL PREFERENCES https://phabricator.haskell.org/settings/panel/emailpreferences/ To: simonpj, Harbormaster

2 1

Better X87
by Thomas Jakway 21 Oct '16

21 Oct '16

I was looking through compiler/nativeGen/X86/Instr.hs <https://github.com/ghc/ghc/blob/master/compiler/nativeGen/X86/Instr.hs#L71> and it's pretty hard not to notice the (hilarious) diatribe about the horror that is x87. git log -p says this was apparently written in 2009 by Ben.Lippmeier(a)anu.edu.au (92ee78e03c3670f56ebbbbfb0f67a00f9ea1305f). Since this has survived in X86/ all this time I'm guessing this is still an issue (another guess: we've gotten by because of SSE?). Is there any interest in improving x87 code generation? And if so, has anyone tried before? According to the comment there seems to be room for improvement. Sadly I don't think x87 is going away any time soon. -Thomas Jakway

4 4

Compact regions in users guide
by Ben Gamari 21 Oct '16

21 Oct '16

Hello Compact Regions authors, It occurs to me that the compact regions support that is due to be included in GHC 8.2 is lacking any discussion in the users guide. At very least we should have a mention in the release notes (this is one of the major features of 8.2, afterall) and a brief overview of the feature elsewhere. It's a bit hard saying where the overview would fit (parallel.rst is an option, albeit imperfect; glasgow_exts.rst is another). I'll leave this up to you. I've opened #12413 [1] to track this task. Do you suppose one of you could take a few minutes to finish this off? Thanks! Cheers, - Ben [1] https://ghc.haskell.org/trac/ghc/ticket/12413

2 1

Perf on T10858
by Simon Peyton Jones 20 Oct '16

20 Oct '16

I’m getting this on HEAD on my Linux box (64 bit) cd "./perf/T10858.run" && "/5playpen/simonpj/HEAD-2/inplace/test spaces/ghc-stage2" -c T10858.hs -dcore-lint -dcmm-lint -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups -dno-debug-output -O +RTS -V0 -tT10858.comp.stats --machine-readable -RTS bytes allocated value is too low: (If this is because you have improved GHC, please update the test so that GHC doesn't regress again) Expected T10858(normal) bytes allocated: 241655120 +/-8% Lower bound T10858(normal) bytes allocated: 222322710 Upper bound T10858(normal) bytes allocated: 260987530 Actual T10858(normal) bytes allocated: 221938928 Deviation T10858(normal) bytes allocated: -8.2 % Does anyone else? It’s good – but why isn’t Harbormaster complaining? Simon

3 2