- User Since
- Jun 8 2014, 1:59 AM (141 w, 3 d)
necessary, but not sufficient perhaps. I look forward to seeing what else you find!
- Add a test
- Suggest -rdynamic in the docs
@bgamari I'd be happy to change the flag name if you can think of anything better. -flink-whole-static-hs-libs maybe?
Is this a bunch of patches squashed together? Could they be put into separate diffs? It would be nicer to keep the changes separate in the history, and it's not really possible to review this.
Well ok. I did leave it there deliberately, but I agree it's weak.
Mon, Feb 20
Wed, Feb 15
Tue, Feb 14
Mon, Feb 13
Sat, Feb 11
Yeah, this code is probably over 20 years old and hasn't had much love since then. Thanks for cleaning it up!
In general I support having this, however as it stands there are too many problems.
- It needs to work for all configurations (TABLES_NEXT_TO_CODE, profiling)
- There's a great deal of overlap with existing code in other places (the GHCi debugger, GHCi itself, and probably bits of the code generator). If this is being brought into the GHC tree we should at least make some effort to de-dup so that we don't have yet more dependencies on representations.
Fri, Feb 10
Yep, LGTM, as long as the IO tests all pass.
Thu, Feb 9
Fine modulo one wibble below.
Thu, Feb 2
Only had time to take a brief look I'm afraid, but my thought was why can't we pass the State# token that we already have to noDuplicate#, rather than grabbing a new one with runRW#?
It looks like this was committed despite the performance regressions in perf/compiler/all.T. What's the plan here?
Wed, Feb 1
I'll run make test with the GHCi way both with and without computed gotos and see which takes longer.
Fri, Jan 27
These two changes would have been better separated, I think.
I don't have strong opinions on this, but I do remember libstdc++ being a problem in the past. What does this do to the ghcilink003 test?
Thu, Jan 26
While you're noodling with lazy ST, maybe I could draw your attention to Trac #11760, just in case you feel like fixing it?
I'd still really like to see some measurements, since there's a complexity cost to doing this.
Wed, Jan 25
First pass, I didn't read everything but I have a bunch of questions that should help me understand it. It actually doesn't seem that complicated, it's just plumbing things around, so the main thing for me is being able to understand the data types and the transformations, I think that could be clearer in the comments.
set the flag too
Jan 23 2017
It replaces a single dispatch point with multiple ones. This improves CPU branch prediction.
Have you measured the difference? Did you look at the generated asm to check that it's doing the right thing?
This will need significant reworking, it needs to be rebuilt on top of the support for NUMA in the block allocator. I'd be happy to advise if anyone is excited about reviving it.
Jan 19 2017
Jan 18 2017
I wouldn't want to speculate about how to fix the graph-colouring allocator before we have some concrete evidence about what the problem is.
Jan 16 2017
We should fix the .gitignore too, I have no idea why it has foo*
Jan 13 2017
Looks like that +20% size increase deserves investigating too.
one more :ghc-flag:
Thanks for the review @dfeur!
Jan 12 2017
Jan 11 2017
Just a couple of wibbles below
The compiler performance regressions are a bit worrying. We should understand those before committing. Perhaps removing the LNE analysis from CoreToStg will help?
Yes, I took a look at this yesterday, I think it's ok. After a bit of thought I realised that plusForeignPtr :: ForeignPtr a -> Int -> ForeignPtr b can't do anything to the finalizer, because it's pure. In the same way that castForeignPtr can't have any effect on the finalizer. But you're probably right that we ought to document it.
Jan 10 2017
Yes, this is probably fine
Jan 9 2017
Looks nice! Just a few changes requested below.
Only skimmed the patch, but it looks sensible to me.
Jan 6 2017
Jan 4 2017
The GHCi changes look fine, so I'm happy with this once @simonpj's latest CSE suggestion is addressed.
Why do we want this rather than having generic barrier primops? If we added a few barriers instead, we'd have fewer primops I think. There's already the MO_WriteBarrier MachOp, which doesn't have a corresponding Haskell-level primop, but it could.
Dec 21 2016
Are the nofib results in the summary still valid? A 15% reduction in binary size would be amazing.
Yes, I think this should be fine now, just one suggestion below.
Dec 19 2016
Does this mean we can't unwind past an stg_stack_underflow_frame? That would be sad! The next stack chunk is pointed to by the frame, so it should be easy enough to unwind, I'd have thought.
This all looks fine to me. I like the two-stage merging followed by validation, I like the reverse dependency index to speed up the recursive removal., and I like the more informative error messages.
GHCi changes are still pending here (see https://phabricator.haskell.org/D2605#79832)
We'll need another change: currently stats are only collected when +RTS -T is on, so we'll need to collect stats unconditionally. I don't think there's any reason not to.
Dec 16 2016
I'm sad that we need the module name here and not just the package name, but if you follow it through it's due to Trac #8696. I still think we should do something more clever though, it's a silly reason to lose an important optimisation.
howToAccessLabel only cares about the package (or at least it should, there's something strange going on with HpcTickLabels, but I'm sure it doesn't affect Cmm compilation).
Dec 15 2016
It shouldn't have any effect on profiled builds, because we're not running any profiled code, only compiling it. Or perhaps I misunderstood the question?
Dec 14 2016
I think we need to change the way the grace works, see below.
Looks like a step in the right direction, thanks for doing this.
Looks ok to me, I think @niteria probably has thoughts.
Dec 13 2016
@RyanGlScott sorry, the reason is that we have instance Binary ExitCode in libraries/ghci (which requires Generic ExitCode), and our binary package doesn't have Binary ExitCode. Maybe we need to update binary?
Maybe because we derive Generic first for the outer type and that requires Generic ExitCode?
@RyanGlScott we derive Binary for lots of things, here's where we use ExitCode: https://phabricator.haskell.org/diffusion/GHC/browse/master/libraries/ghci/GHCi/Message.hs;24f6bec94411aa6c39a2c94ce5154ffe96ae330f$351
Ugh, I remember working on this before, when I wrote the X86_INIT_FPU stuff, it's all horrible.
Yes we'll need to relax the lower bound to make this compile. The requirement is to compile with the past two major releases, which for master means 7.10+.