- ghc-devs - Haskell.org

RE: [commit: ghc] master: Respect the ordering of -package directives (574ccfa)
by Simon Peyton-Jones 02 Dec '13

02 Dec '13

Patrick Thanks! When you do something subtle like change 'foldM' to 'foldrM', could you add a comment to point out the subtlety? After all, by definition this was something that someone else missed. Simon | -----Original Message----- | From: ghc-commits [mailto:ghc-commits-bounces@haskell.org] On Behalf Of | git(a)git.haskell.org | Sent: 02 December 2013 02:39 | To: ghc-commits(a)haskell.org | Subject: [commit: ghc] master: Respect the ordering of -package | directives (574ccfa) | | Repository : ssh://git@git.haskell.org/ghc | | On branch : master | Link : | http://ghc.haskell.org/trac/ghc/changeset/574ccfa231ca05d03d1da9d31e5bc | 81e74cc5e1e/ghc | | >--------------------------------------------------------------- | | commit 574ccfa231ca05d03d1da9d31e5bc81e74cc5e1e | Author: Patrick Palka <patrick(a)parcs.ath.cx> | Date: Tue Nov 26 11:46:59 2013 -0500 | | Respect the ordering of -package directives | | | >--------------------------------------------------------------- | | 574ccfa231ca05d03d1da9d31e5bc81e74cc5e1e | compiler/main/Packages.lhs | 15 ++++++++------- | 1 file changed, 8 insertions(+), 7 deletions(-) | | diff --git a/compiler/main/Packages.lhs b/compiler/main/Packages.lhs | index aefb536..df5eddb 100644 | --- a/compiler/main/Packages.lhs | +++ b/compiler/main/Packages.lhs | @@ -59,6 +59,7 @@ import System.FilePath as FilePath | import qualified System.FilePath.Posix as FilePath.Posix | import Control.Monad | import Data.Char (isSpace) | +import Data.Foldable (foldrM) | import Data.List as List | import Data.Map (Map) | import qualified Data.Map as Map | @@ -980,7 +981,7 @@ getPreloadPackagesAnd dflags pkgids = | preload = preloadPackages state | pairs = zip pkgids (repeat Nothing) | in do | - all_pkgs <- throwErr dflags (foldM (add_package pkg_map ipid_map) | preload pairs) | + all_pkgs <- throwErr dflags (foldrM (add_package pkg_map ipid_map) | preload pairs) | return (map (getPackageDetails state) all_pkgs) | | -- Takes a list of packages, and returns the list with dependencies | included, | @@ -1003,15 +1004,15 @@ closeDepsErr :: PackageConfigMap | -> Map InstalledPackageId PackageId | -> [(PackageId,Maybe PackageId)] | -> MaybeErr MsgDoc [PackageId] | -closeDepsErr pkg_map ipid_map ps = foldM (add_package pkg_map | ipid_map) [] ps | +closeDepsErr pkg_map ipid_map ps = foldrM (add_package pkg_map | ipid_map) [] ps | | -- internal helper | add_package :: PackageConfigMap | -> Map InstalledPackageId PackageId | - -> [PackageId] | -> (PackageId,Maybe PackageId) | + -> [PackageId] | -> MaybeErr MsgDoc [PackageId] | -add_package pkg_db ipid_map ps (p, mb_parent) | +add_package pkg_db ipid_map (p, mb_parent) ps | | p `elem` ps = return ps -- Check if we've already added this | package | | otherwise = | case lookupPackage pkg_db p of | @@ -1019,12 +1020,12 @@ add_package pkg_db ipid_map ps (p, mb_parent) | missingDependencyMsg mb_parent) | Just pkg -> do | -- Add the package's dependents also | - ps' <- foldM add_package_ipid ps (depends pkg) | + ps' <- foldrM add_package_ipid ps (depends pkg) | return (p : ps') | where | - add_package_ipid ps ipid@(InstalledPackageId str) | + add_package_ipid ipid@(InstalledPackageId str) ps | | Just pid <- Map.lookup ipid ipid_map | - = add_package pkg_db ipid_map ps (pid, Just p) | + = add_package pkg_db ipid_map (pid, Just p) ps | | otherwise | = Failed (missingPackageMsg str <> missingDependencyMsg | mb_parent) | | | _______________________________________________ | ghc-commits mailing list | ghc-commits(a)haskell.org | http://www.haskell.org/mailman/listinfo/ghc-commits

2 1

Re: [commit: testsuite] master: Explain skipping compiler-perf tests when debugging is on (14958ae)
by Simon Marlow 02 Dec '13

02 Dec '13

Actually this isn't quite right. compiler_debugged() is true when GhcDebugged=YES in the build system, and GhcDebugged controls whether GHC is compiled with -debug or not. That is, compiler_debugged() tells us whether the compiler was compiled with -debug, not -DDEBUG. The -debug flag links in the debug RTS and doesn't change allocation or heap residency, whereas -DDEBUG enables assertions in the compiler and *does* change allocation and heap residency. It looks like we need another predicate to tell us whether GHC was compiled with -DDEBUG or not. Would you mind backing out these changes in the meantime? Cheers, Simon On 28/11/13 15:55, git(a)git.haskell.org wrote: > Repository : ssh://git@git.haskell.org/testsuite > > On branch : master > Link : http://ghc.haskell.org/trac/ghc/changeset/14958ae02b99bafc06288493d186b9313… > >> --------------------------------------------------------------- > > commit 14958ae02b99bafc06288493d186b9313ee8c4b9 > Author: Joachim Breitner <mail(a)joachim-breitner.de> > Date: Thu Nov 28 15:56:16 2013 +0000 > > Explain skipping compiler-perf tests when debugging is on > > >> --------------------------------------------------------------- > > 14958ae02b99bafc06288493d186b9313ee8c4b9 > driver/testlib.py | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/driver/testlib.py b/driver/testlib.py > index bdb91b8..8c8c60d 100644 > --- a/driver/testlib.py > +++ b/driver/testlib.py > @@ -291,6 +291,8 @@ def _compiler_stats_num_field( name, opts, field, expecteds ): > if field in opts.compiler_stats_range_fields: > framework_fail(name, 'duplicate-numfield', 'Duplicate ' + field + ' num_field check') > > + # Compiler performance numbers change when debugging is on, making the results > + # useless and confusing. Therefore, skip if debugging is on. > if compiler_debugged(): > skip(name, opts) > > > _______________________________________________ > ghc-commits mailing list > ghc-commits(a)haskell.org > http://www.haskell.org/mailman/listinfo/ghc-commits >

2 3

Re: Getting Involved
by Jan Stolarek 02 Dec '13

02 Dec '13

Herbert, Austin, right, I didn't notice link to Newcomers is on the "Working on GHC" page. I rearranged it slightly by reversing first three steps. So link to Newcomers is now first on the list and I think it might stay that way. Link to "Working on GHC" is also in the second paragraph of the main wiki page, so I think we're good. As for the sidebar I'm using it on a regular basis although I feel some links could be removed: Documentation -> Repositories; Debugging; View Tickets -> By Milestone; By OS; By Architecture; But that's only my opinion - I'm CCing ghc-devs so others can chime in. If we could somehow track how often each link is used that could be a valuable information. Not sure if it can be done. Janek Dnia niedziela, 1 grudnia 2013, Herbert Valerio Riedel napisał: > On 2013-12-01 at 09:00:21 +0100, Jan Stolarek wrote: > >> perhaps a direct link in the sidebar or maybe even a link on the front > >> page in Developer Documentation section would be appropriate! > > > > I added link to Newcomers page at the main page of Commentary. > > btw, I'm confused a bit... there's already a rather obvious > > "Joining In"->"Working on GHC" link in the side-bar, which points to > > https://ghc.haskell.org/trac/ghc/wiki/WorkingConventions > > which then points you to the steps that are expected from newcomers, > including the 'Newcomers' page as the 3rd step; once could maybe just > add more emphasis to the 3rd step so that it visually stands out better > (e.g. via [2] or [3]) > > I'm just worried that cluttering the side-bar[1] with a redundant link > may make it less useful. > > > Don't have the permissions to add it to the sidebar though. > > ...the reason is btw, that it's a hacked in HTML fragment that's part of > the custom Trac stylesheet mod for GHC; it can't be edited via the > web-UI. > > > > [1]: which, personally, I find kinda overloaded and redundant which > makes me wonder if it's really that useful in its current form; > > > [2]: https://ghc.haskell.org/trac/ghc/wiki/WikiBoxes > > [3]: https://ghc.haskell.org/trac/ghc/wiki/WikiIcons

2 1

build errors
by Kazu Yamamoto 29 Nov '13

29 Nov '13

Hi, I cannot build GHC head on Linux and Mac (Mavericks) today: compiler/typecheck/TcEvidence.lhs:152:16: Not in scope: data constructor `ASSERT2' compiler/typecheck/TcEvidence.lhs:174:5: Not in scope: data constructor `ASSERT2' compiler/typecheck/TcEvidence.lhs:489:15: Not in scope: data constructor `ASSERT2' compiler/typecheck/TcEvidence.lhs:712:5: Not in scope: data constructor `ASSERT2' make[1]: *** [compiler/stage1/build/TcEvidence.o] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [all] Error 2 --Kazu

4 5

RE: [commit: testsuite] master: Summarily skip compiler_perf tests when DEBUG is on (715e313)
by Simon Peyton-Jones 28 Nov '13

28 Nov '13

Good! but please add a comment to explain why! The code is fully of mysterious un-commented things like this. Simon | -----Original Message----- | From: ghc-commits [mailto:ghc-commits-bounces@haskell.org] On Behalf Of | git(a)git.haskell.org | Sent: 28 November 2013 14:19 | To: ghc-commits(a)haskell.org | Subject: [commit: testsuite] master: Summarily skip compiler_perf tests | when DEBUG is on (715e313) | | Repository : ssh://git@git.haskell.org/testsuite | | On branch : master | Link : | http://ghc.haskell.org/trac/ghc/changeset/715e313d84f4e919e12c6bba6cd646 | e488cffa74/testsuite | | >--------------------------------------------------------------- | | commit 715e313d84f4e919e12c6bba6cd646e488cffa74 | Author: Joachim Breitner <mail(a)joachim-breitner.de> | Date: Thu Nov 28 14:17:35 2013 +0000 | | Summarily skip compiler_perf tests when DEBUG is on | | by ensuring this in the test runner. | | | >--------------------------------------------------------------- | | 715e313d84f4e919e12c6bba6cd646e488cffa74 | driver/testlib.py | 3 +++ | 1 file changed, 3 insertions(+) | | diff --git a/driver/testlib.py b/driver/testlib.py | index ed7d02f..bdb91b8 100644 | --- a/driver/testlib.py | +++ b/driver/testlib.py | @@ -291,6 +291,9 @@ def _compiler_stats_num_field( name, opts, field, | expecteds ): | if field in opts.compiler_stats_range_fields: | framework_fail(name, 'duplicate-numfield', 'Duplicate ' + field | + ' num_field check') | | + if compiler_debugged(): | + skip(name, opts) | + | for (b, expected, dev) in expecteds: | if b: | opts.compiler_stats_range_fields[field] = (expected, dev) | | _______________________________________________ | ghc-commits mailing list | ghc-commits(a)haskell.org | http://www.haskell.org/mailman/listinfo/ghc-commits

1 0

RE: [commit: testsuite] master: Update some perf results for master (4afbacc)
by Simon Peyton-Jones 28 Nov '13

28 Nov '13

Wait. Are you really increasing "bytes allocated" from 97M to 237M? That's a big change, and in an important number. Do you know why? Simon | -----Original Message----- | From: ghc-commits [mailto:ghc-commits-bounces@haskell.org] On Behalf Of | git(a)git.haskell.org | Sent: 28 November 2013 12:36 | To: ghc-commits(a)haskell.org | Subject: [commit: testsuite] master: Update some perf results for | master (4afbacc) | | Repository : ssh://git@git.haskell.org/testsuite | | On branch : master | Link : | http://ghc.haskell.org/trac/ghc/changeset/4afbacc299d843eb632df1f71eee2 | c91746c85a8/testsuite | | >--------------------------------------------------------------- | | commit 4afbacc299d843eb632df1f71eee2c91746c85a8 | Author: Joachim Breitner <mail(a)joachim-breitner.de> | Date: Thu Nov 28 12:36:15 2013 +0000 | | Update some perf results for master | | | >--------------------------------------------------------------- | | 4afbacc299d843eb632df1f71eee2c91746c85a8 | tests/perf/compiler/all.T | 11 ++++++----- | tests/perf/space_leaks/all.T | 2 +- | 2 files changed, 7 insertions(+), 6 deletions(-) | | diff --git a/tests/perf/compiler/all.T b/tests/perf/compiler/all.T | index a580bf9..759b5d0 100644 | --- a/tests/perf/compiler/all.T | +++ b/tests/perf/compiler/all.T | @@ -344,10 +344,11 @@ test('T5837', | test('T6048', | [ only_ways(['optasm']), | compiler_stats_num_field('bytes allocated', | - [(wordsize(32), 48887164, 10), | - # prev: 38000000 (x86/Linux) | - # 2012-10-08: 48887164 (x86/Linux) | - (wordsize(64), 97247032, 10)]) | - # 18/09/2012 97247032 amd64/Linux | + [(wordsize(32), 48887164, 10), | + # prev: 38000000 (x86/Linux) | + # 2012-10-08: 48887164 (x86/Linux) | + (wordsize(64), 237077056, 10)]) | + # 18/09/2012 97247032 amd64/Linux | + # 2013-11-26: 237077056 amd64/Linux | ], | compile,['']) | diff --git a/tests/perf/space_leaks/all.T | b/tests/perf/space_leaks/all.T | index a1fd641..ac60c8f 100644 | --- a/tests/perf/space_leaks/all.T | +++ b/tests/perf/space_leaks/all.T | @@ -6,7 +6,7 @@ test('space_leak_001', | # 5 (x86/Linux) | [stats_num_field('peak_megabytes_allocated', (4, 1)), | stats_num_field('max_bytes_used', | - [(wordsize(64), 440000, 10), | + [(wordsize(64), 440000, 15), | # 440224 (amd64/Linux) | # 417016 (x86/OS X) | # 415672 (x86/Windows) | | _______________________________________________ | ghc-commits mailing list | ghc-commits(a)haskell.org | http://www.haskell.org/mailman/listinfo/ghc-commits

3 6

Re: [commit: testsuite] master: Higher residency in Haddock (15c09a3)
by Simon Marlow 28 Nov '13

28 Nov '13

On 22/11/13 16:43, git(a)git.haskell.org wrote: > Repository : ssh://git@git.haskell.org/testsuite > > On branch : master > Link : http://ghc.haskell.org/trac/ghc/changeset/15c09a332cd24e5d6c4c9a0ede9b151c9… > >> --------------------------------------------------------------- > > commit 15c09a332cd24e5d6c4c9a0ede9b151c9a095f66 > Author: Simon Peyton Jones <simonpj(a)microsoft.com> > Date: Fri Nov 22 16:42:43 2013 +0000 > > Higher residency in Haddock > > I think there really is a slight worsening in the situation here, but > it needs someone to build a profiled compiler and take a proper look. Maybe make a ticket for the next milestone then? I'm getting slightly worried that we keep pushing these performance bounds up without really investigating why. The great thing about the perf tests is that they catch something immediately, rather than us having to bisect it or do lengthy profiling investigations later. What should we do about this? I realise it's a pain when you've got a working patch and the only thing holding it up is some random perf test that seems to be completely unrelated. Let's have a brainstorm on how we can improve things. What tools can we build to help? Maybe this is a good way that new contributors could get involved. Here's one idea off the top of my head: let's make a way to take heap samples at deterministic points during compilation, say between phases. Then make a tool to compare profiles made this way, so that we can say what changed between two compiler versions. I'm pretty sure this will spot leaks pretty quickly, and tell you something about what is leaking (which constructors). I think we've accrued some bloat recently. I had to increase the memory in my VM because it ceased to be able to build HEAD at some point. Cheers, Simon

2 2

worker-wrapper correctness
by Joachim Breitner 28 Nov '13

28 Nov '13

Hi, I first planned to post this to https://ghc.haskell.org/trac/ghc/ticket/1600 but it might be of wider interest, and is not only related to CPR. SPJ pointed me to this¹ paper by Graham Hutton (thanks for that). It lists three sufficient conditions for the w/w-wrapper to be semantically correct: (1) wrap ◦ unwrap = id (2) wrap ◦ unwrap ◦ body = body (3) fix (wrap ◦ unwrap ◦ body) = fix body The first one is often not true when we do w/w in GHC. The third one seems to be out of reach: Arguing about a recursive function in this way smells like solving the halting problem. So it seems that we should be aiming for (2) wrap ◦ unwrap ◦ body = body With this in mind, setting the CPR info of recursive things to ⊤, i.e. “we don’t know anything” _when analyzing their RHS_ should do the right thing: If we assume nothing about the argument of `body`, the result will hold for all, which precisely is `wrap ◦ unwrap ◦ body = body`. And this should be the case not only for CPR, but for anything that does a w/w-transformation, i.e. Demand analysis as well. But what about strictness analysis? Clearly, for f :: Int -> Int -> Int f n m = if n == 0 then m else f (n-1) m we want to know (as we do in 7.6. and in HEAD) that it is strict in m... Interesting fact here: GHC-7.6 will find this worker signature wf :: Int# -> Int -> Int (which is compatible with condition (2)), but GHC HEAD will do wf :: Int# -> Int# -> Int# where equation (2) is no longer holds. In this case (3) happens to hold (at least wrt to a semantics that does not distinguish non-termination of other kinds of ⊥). Conclusion: The correctness of GHC’s w/w-transformation is currently not formally well supported, and it only works due to delicate and non-obvious² interaction between the analysis and the transformation (see comment 20 of #1600 for an example how easily it breaks). But if we systematically ensure the correctness, by always guaranteeing (2), we are losing existing chances for optimization. Some more less consistent thoughts: There seems to be a conceptual difference between the CPR w/w-transformation and the others: Unboxing arguments pulls work out of the worker to be done _before_ it is being called. Clearly, this will never make the recursive call to the work diverge, unless it would diverge anyways. And if the work pulled out diverges (e.g. if m = undefined in the example above), then either the worker would have diverged (but that is fine) or, assuming the analysis is correct, would have performed the work. So for these transformations it is ok to use the full information in the recursive case. But the CPR transformation is different: The w/w transformation forces the evaluation of the result of the recursive call and in that sense moves work into it. And if this again triggers work to be moved into the next recursive calls, we can cause divergence. So by this handwavy argument, we are fine for unboxing arguments and strictness analysis, but for CPR we really should forget what we know about the f when analyzing f’s body. Thanks for your attention, Joachim ¹ http://www.cs.nott.ac.uk/~gmh/wrapper.pdf ² to me at least – I might be missing something -- Joachim “nomeata” Breitner mail(a)joachim-breitner.de • http://www.joachim-breitner.de/ Jabber: nomeata(a)joachim-breitner.de • GPG-Key: 0x4743206C Debian Developer: nomeata(a)debian.org

1 0

RTS warnings
by Gabor Greif 27 Nov '13

27 Nov '13

Hi all! I spottes these warnings in today's $ make INTEGER_LIBRARY=integer-simple V=0 build: rts/Stats.c: In function 'stat_exit': rts/Stats.c:695:0: warning: format '%16zu' expects type 'size_t', but argument 2 has type 'unsigned int' rts/sm/Storage.c: In function 'flushExec': rts/sm/Storage.c:1294:0: warning: implicit declaration of function '__builtin___clear_cache' rts/sm/Storage.c:1294:0: warning: nested extern declaration of '__builtin___clear_cache' this is my "GCC": ggreif$ gcc --version i686-apple-darwin11-llvm-gcc-4.2 (GCC) 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2336.11.00) Copyright (C) 2007 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Cheers, Gabor PS: there is also this low-hanging fruit: libraries/containers/Data/IntSet/Base.hs:1196:17: Warning: In the use of 'bitSize' (imported from Data.Bits): Deprecated: "Use bitSizeMaybe or finiteBitSize instead"

2 1

Cannot instantiate unification variable
by Kazu Yamamoto 26 Nov '13

26 Nov '13

Hi, Recent GHC head on 32bit Linux cannot build conduit-1.0.9.2. (See the attached log.) A while ago, GHC head on 32bit Linux could build it. The same GHC head on 64bit Linux can build it. Michael Snoyman suggested me to add ImpredicativeTypes. When I added it to the Internal.hs, GHC head on 32bit Linux can build it. And he said to other released GHCs can build it. Actually, GHC 7.4.2 on 32bit Linux can build it. He believes that this is a bug of GHC head (on 32bit Linux). Any information? Note that conduit-1.0.9.3 already includes ImpredicativeTypes. --Kazu Building conduit-1.0.9.2... Preprocessing library conduit-1.0.9.2... [2 of 8] Compiling Data.Conduit.Internal ( Data/Conduit/Internal.hs, dist/build/Dat a/Conduit/Internal.o ) Data/Conduit/Internal.hs:211:105: Cannot instantiate unification variable ‛b0’ with a type involving foralls: (forall a. m1 a -> n1 a) -> ConduitM i o m1 b1 -> ConduitM i o n1 b1 Perhaps you want ImpredicativeTypes In the expression: GHC.Exts.coerce (hoist :: (forall (a :: *). m a -> n a) -> Pipe i i o () m b -> Pipe i i o () n b) :: forall (m :: * -> *) (b :: *) (n :: * -> *). Monad m => (forall (a :: *). m a -> n a) -> ConduitM i o m b -> ConduitM i o n b In an equation for ‛hoist’: hoist = GHC.Exts.coerce (hoist :: (forall (a :: *). m a -> n a) -> Pipe i i o () m b -> Pipe i i o () n b) :: forall (m :: * -> *) (b :: *) (n :: * -> *). Monad m => (forall (a :: *). m a -> n a) -> ConduitM i o m b -> ConduitM i o n b

2 1