- User Since
- Oct 6 2014, 1:50 PM (258 w, 4 d)
Apr 30 2018
Jul 27 2017
Mar 7 2017
Hmm, I'll have to find time to take a look at the updated nofib results. There really shouldn't be any increases in binary size (or, ideally, in allocations either).
Feb 28 2017
Feb 27 2017
- sigh, fragile tests
- avoid unnecessary tuples
- disable preemptive floating at -O0
- don't pre-emptively float primitive strings
- clean up code duplication in cseBind
- add a Note explaining why preInlineUnconditionally must guard against inlining string literals.
Good point, I suppose you're right. Might be worth a spot check with a simple example to make sure the deforestation happens?
Hmm, I'm not sure about this. This patch will force us to unpack the Module strings every time they're used, which would be memoized in the current code.
Feb 25 2017
As of the latest patch, the nofib Size increases are gone and there's a single 0.1% Alloc increase.
fix CSE to maintain Core invariant
preInlineUnconditionally was re-inlining the floated primitive strings
- preemptively float primitive strings too (see the last paragraph of the Note)
- we can do CSE on top-level primitive strings
- address comments
- float primitive strings out too
- expand the Note to explain why FloatOut isn't the answer here
- make T10844 more robust
Feb 24 2017
Feb 6 2017
Feb 4 2017
@angerman perhaps not, but it's much more pleasant to work with Core than with HsSyn. I think we should avoid pushing plugin authors (and API clients) towards dealing with the giant types of HsSyn and friends. It's better to keep that complexity internal to GHC.
If I understand correctly, there should be no increase in code size. We are only keeping the binders alive inside the desugarer, they will eventually be dropped in the Core2Core pipeline.
Feb 3 2017
appease -Werror again
Feb 1 2017
@dfeuer good point re unsafePerformIO. As a quick sanity check I tried compiling the following with -O2.
Jan 31 2017
use original activation for workers
make sure noinline worker is never inlined, fix tests
@simonmar the +17% binary size for transform is indeed concerning,
I've done some digging and found that the issue can be reproduced on
HEAD just by adding the CONLIKE pragma to unpackCString#, somehow
this is causing GHC to duplicate a ton of code. I spent some time
yesterday trying to minimize transform and came up with the following.
Jan 30 2017
I was unsure about including the test at first, since GHC already avoided reboxing the Int on the safe paths. But then I noticed that this patch allows GHC to eliminate the Int entirely on the error path, so I think it is in fact useful as a regression test.
Jan 21 2017
Jan 17 2017
Sure, I'll take a look this evening. Thanks for your patience!
Sorry, I've been having trouble properly minimizing the example, but I think I finally got it.
Jan 16 2017
@simonpj sorry to be so slow.
Dec 28 2016
Ok, I've isolated the largest remaining allocation increase to the way GHC.Arr.index (specifically the Int specialization) is inlined.
@simonpj I believe I have traced some of the increased allocations to GHC.Arr (specifically the array function). Looking at the simplified Core I see something a bit odd.
Dec 27 2016
@simonmar no, those nofib results are quite old. I'll update the summary with the final results once I've sorted out the last issues, but the current summary is:
Dec 20 2016
@bgamari thanks for the prod. There are two things preventing this patch from being ready.
Dec 14 2016
Dec 5 2016
Oct 2 2016
@akio sure, would you mind pinging me when you re-post? I have another patch that depends on this one.
Sep 30 2016
@akio there are also three failing compiler perf tests, i.e. GHC is too slow. I noticed that GHC is spending a lot more time in the first simplifier phase on those tests, perhaps the two are related?
fix failing tests
Sep 2 2016
Aug 29 2016
On master, the $wxs1 and $wgo1 functions are lifted to top-level binders, and as far as I can tell they *could* be lifted out here too. Curious...
That looks like a place where allocation will change, because each call to $wdraw will allocate a closure for each of those functions. To narrow it down, look at the output of FloatOut, and see why they aren't being floated.
Aug 21 2016
I finally had some time to dig into the regressions a bit, specifically the cse benchmark. The -ticky flag helped me isolate the allocation increases to the draw function.
Jul 31 2016
Address Simon's comments.
Apr 13 2016
Sounds good to me!
Mar 29 2016
split isCallStack into top-level function
Mar 25 2016
Kill *all* inference of CallStacks.
Mar 21 2016
- elaborate the HasCallStack docs with an example of HasCallStack inferrence
- use any in the definition of elemType
break import loop
On second thought, adding another constructor to TopLevelFlag seems overly complex when we already have a way to determine top-level ghci binders, isTopLevel && hscTarget == HscInterpreted
add TopLevelFlag.GhciTopLevel and merge master
Mar 7 2016
Feb 19 2016
- check for top-level-ness based on the TopLevelFlag
- decide whether to quantify over a CallStack in pickQuantifiablePreds
- replace inferred ?callStack::CallStack with HasCallStack (in pickQuantifiablePreds) instead of special casing the printer
Feb 15 2016
Feb 14 2016
good point, we might as well group the keys
I don't have a strong opinion on whether to land this and complicate (slightly) the solver, or keep things as they are. But given the concerns today on haskell-cafe over the beginner-friendliness of HasCallStack, I thought I'd at least offer the patch.
Feb 13 2016
Feb 11 2016
Thanks! The only thing I'd suggest is adding a warning to the IsList instance that fromList . toList won't preserve frozenness, because fromList will always produce an unfrozen stack. I doubt that will ever come up, but if it does it could leave people quite confused!
Feb 9 2016
Ok then let's just print it as a list. We could add an IsList instance too, that seems reasonable.
Right, that's why we expose pushCallStack instead of the constructor :)
I'm not in favor of adding derived instances for CallStack (particularly Show). The implementation is hidden for a reason, so we can make changes in the future if necessary (and we already have!), and the derived instances expose implementation details.
Feb 8 2016
Feb 5 2016
Jan 27 2016
give the HasCallStack alias in the Note, add the API warning to the user's guide as well
fix the testsuite, tie-in HasCallStack in "Note [Overview of implicit CallStacks]", and remove setCallStack as it does not work as intended (the intended function can't be written in Haskell).
@simonpj I warned about the underlying implicit parameter in the haddocks for HasCallStack and CallStack. I didn't mention it in the user's guide because I think the only way you would discover the IP is by browsing the haddocks or source. But I can certainly copy the warning to the user's guide if you think it would be beneficial.
Jan 26 2016
rebase and make a few more tweaks. @bgamari, I'm happy with the docs now (main change was to add a step explaining when GHC infers HasCallStack constraints).
Jan 25 2016
tweak code blocks
A first draft of the revised CallStack docs, comments are very welcome.
Jan 24 2016
Thanks, I'm rewriting this section anyway as part of D1818, so I'll fix the typos there.
Jan 21 2016
@bgamari according to the test file (testsuite/tests/polykinds/T11466.hs) that error is expected, it appears the commit that added the test (395ec414ff21bc37439194bb31a8f764b38b0fca) forgot to add the .stderr :)
@bgamari this will need to be applied to 8.0 as well, fyi.
Jan 20 2016
isCallStackCt :: Ct -> Maybe FastString
Jan 19 2016
The reason we end up with non-canonical constraints in the defaulting phase is that runTcPluginsWanted zonks the simple constraints. We seem to zonk the constraints during defaulting anyway, so I guess this is fine.
clean up the test case
@simonpj, I find it suspicious that non-canonical constraints reach the defaulting stage. There may be a separate issue with the type-checker plugin infrastructure, because this issue arises with a no-op plugin.