gridaphobe (Eric Seidel)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 6 2014, 1:50 PM (258 w, 4 d)

Recent Activity

Apr 30 2018

gridaphobe accepted D4648: errorWithoutStackTrace: omit profiling stack trace (#14970).

makes sense

Apr 30 2018, 1:21 PM

Jul 27 2017

gridaphobe accepted D3795: Ensure that GHC.Stack.callStack doesn't fail.
Jul 27 2017, 9:16 PM
gridaphobe added a comment to D3795: Ensure that GHC.Stack.callStack doesn't fail.

Good catch!

Jul 27 2017, 9:15 PM

Mar 7 2017

gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

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).

Mar 7 2017, 11:04 PM

Feb 28 2017

gridaphobe added inline comments to D1259: Don't inline String literals or CallStacks.
Feb 28 2017, 11:04 AM

Feb 27 2017

gridaphobe updated the diff for D1259: Don't inline String literals or CallStacks.
  • sigh, fragile tests
  • avoid unnecessary tuples
  • disable preemptive floating at -O0
Feb 27 2017, 7:14 PM
gridaphobe added inline comments to D3224: DsBinds: Use Typeable Module binding when desugaring CallStack evidence.
Feb 27 2017, 3:10 PM
gridaphobe updated the diff for D1259: Don't inline String literals or CallStacks.
  • 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.
Feb 27 2017, 2:47 PM
gridaphobe added inline comments to D1259: Don't inline String literals or CallStacks.
Feb 27 2017, 2:31 PM
gridaphobe accepted D3224: DsBinds: Use Typeable Module binding when desugaring CallStack evidence.

Good point, I suppose you're right. Might be worth a spot check with a simple example to make sure the deforestation happens?

Feb 27 2017, 12:28 PM
gridaphobe added a comment to D3224: DsBinds: Use Typeable Module binding when desugaring CallStack evidence.

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 27 2017, 11:05 AM

Feb 25 2017

gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

As of the latest patch, the nofib Size increases are gone and there's a single 0.1% Alloc increase.

Feb 25 2017, 10:35 PM
gridaphobe updated the diff for D1259: Don't inline String literals or CallStacks.

fix CSE to maintain Core invariant

Feb 25 2017, 8:55 PM
gridaphobe updated the diff for D1259: Don't inline String literals or CallStacks.

preInlineUnconditionally was re-inlining the floated primitive strings

Feb 25 2017, 6:32 PM
gridaphobe updated the diff for D1259: Don't inline String literals or CallStacks.
  • preemptively float primitive strings too (see the last paragraph of the Note)
  • we can do CSE on top-level primitive strings
Feb 25 2017, 5:02 PM
gridaphobe updated the diff for D1259: Don't inline String literals or CallStacks.
  • address comments
  • float primitive strings out too
  • expand the Note to explain why FloatOut isn't the answer here
  • make T10844 more robust
Feb 25 2017, 3:10 PM

Feb 24 2017

gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

I've committed a patch to HEAD that makes unpackCString CONLIKE (and documents why). And improves exprIsConApp_maybe.

Does that get you un-glued?

Feb 24 2017, 9:30 AM

Feb 6 2017

gridaphobe added a comment to D3073: Do not drop dead code in the desugarer.

perf.haskell.org reports the absence of significant regressions: https://perf.haskell.org/ghc/#revision/b59c2de7abe3cd4e046f11c3536ba8e7137c4f84

Feb 6 2017, 7:00 PM

Feb 4 2017

gridaphobe added a comment to D3073: Do not drop dead code in the desugarer.

@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.

Feb 4 2017, 7:56 PM
gridaphobe added a comment to D3073: Do not drop dead code in the desugarer.

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 4 2017, 10:36 AM

Feb 3 2017

gridaphobe updated the diff for D3046: Do Worker/Wrapper for NOINLINE things.

appease -Werror again

Feb 3 2017, 12:55 AM

Feb 1 2017

gridaphobe added a comment to D3046: Do Worker/Wrapper for NOINLINE things.

@dfeuer good point re unsafePerformIO. As a quick sanity check I tried compiling the following with -O2.

Feb 1 2017, 10:26 AM

Jan 31 2017

gridaphobe updated the diff for D3046: Do Worker/Wrapper for NOINLINE things.

use original activation for workers

Jan 31 2017, 5:31 PM
gridaphobe added inline comments to D3046: Do Worker/Wrapper for NOINLINE things.
Jan 31 2017, 5:22 PM
gridaphobe updated the diff for D3046: Do Worker/Wrapper for NOINLINE things.

make sure noinline worker is never inlined, fix tests

Jan 31 2017, 4:53 PM
gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

@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 31 2017, 2:00 PM

Jan 30 2017

gridaphobe updated the diff for D3046: Do Worker/Wrapper for NOINLINE things.

appease -Werror

Jan 30 2017, 5:32 PM
gridaphobe added a reviewer for D3046: Do Worker/Wrapper for NOINLINE things: simonpj.
Jan 30 2017, 3:29 PM
gridaphobe added a comment to D3046: Do Worker/Wrapper for NOINLINE things.

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 30 2017, 3:29 PM
gridaphobe retitled D3046: Do Worker/Wrapper for NOINLINE things from to Do Worker/Wrapper for NOINLINE things.
Jan 30 2017, 3:25 PM

Jan 21 2017

gridaphobe added a comment to D2999: base: Improve Prelude.read error message.

Great idea!

Jan 21 2017, 6:41 PM

Jan 17 2017

gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

Sure, I'll take a look this evening. Thanks for your patience!

Jan 17 2017, 5:44 PM
gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

Sorry, I've been having trouble properly minimizing the example, but I think I finally got it.

Jan 17 2017, 4:25 PM

Jan 16 2017

gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

@simonpj sorry to be so slow.

Jan 16 2017, 11:39 PM

Dec 28 2016

gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

Ok, I've isolated the largest remaining allocation increase to the way GHC.Arr.index (specifically the Int specialization) is inlined.

Dec 28 2016, 4:09 PM
gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

@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 28 2016, 1:17 PM

Dec 27 2016

gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

@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 27 2016, 6:06 PM

Dec 20 2016

gridaphobe added a parent revision for D1259: Don't inline String literals or CallStacks: D2605: Allow top-level string literals in Core (#8472).

@bgamari thanks for the prod. There are two things preventing this patch from being ready.

Dec 20 2016, 12:08 PM
gridaphobe added a child revision for D2605: Allow top-level string literals in Core (#8472): D1259: Don't inline String literals or CallStacks.
Dec 20 2016, 12:08 PM

Dec 14 2016

gridaphobe added inline comments to D2605: Allow top-level string literals in Core (#8472).
Dec 14 2016, 10:19 PM

Dec 5 2016

gridaphobe added inline comments to D2605: Allow top-level string literals in Core (#8472).
Dec 5 2016, 4:21 AM

Oct 2 2016

gridaphobe added a comment to D2554: Float primitive string literals to the top.

@akio sure, would you mind pinging me when you re-post? I have another patch that depends on this one.

Oct 2 2016, 11:13 PM

Sep 30 2016

gridaphobe added a comment to D2554: Float primitive string literals to the top.

@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?

Sep 30 2016, 8:55 PM
gridaphobe updated the diff for D2554: Float primitive string literals to the top.

fix failing tests

Sep 30 2016, 4:56 PM
gridaphobe updated the Trac tickets for D2554: Float primitive string literals to the top.
Sep 30 2016, 3:13 PM

Sep 2 2016

gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

Why would GHC single out the first element? And regardless, why would this lead to increased allocations when the entire tuple should be evaluated at most once?

That's odd, but should make absolutely no difference. If you do -ddump-prep it'll probably look identical.

I'm puzzled why ALL of those strings aren't being allocated at top level, thus

ds7 = unpackCString# "[-"#

etc. Why do we end up with nested bindings at all?

Sep 2 2016, 4:40 PM

Aug 29 2016

gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

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 29 2016, 7:17 PM

Aug 21 2016

gridaphobe added a comment to D1259: Don't inline String literals or CallStacks.

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.

Aug 21 2016, 6:50 PM

Jul 31 2016

gridaphobe updated the diff for D1259: Don't inline String literals or CallStacks.

Address Simon's comments.

Jul 31 2016, 2:41 PM

Apr 13 2016

gridaphobe added a comment to D2106: utils: Provide CallStack to expectJust.

Sounds good to me!

Apr 13 2016, 10:09 AM

Mar 29 2016

gridaphobe updated the diff for D1912: Don't infer CallStacks.
split isCallStack into top-level function
Mar 29 2016, 12:27 PM · GHC

Mar 25 2016

gridaphobe retitled D1912: Don't infer CallStacks from Don't infer CallStacks at the top-level to Don't infer CallStacks.
Mar 25 2016, 12:08 PM · GHC
gridaphobe updated the diff for D1912: Don't infer CallStacks.

Kill *all* inference of CallStacks.

Mar 25 2016, 12:04 PM · GHC

Mar 21 2016

gridaphobe updated the diff for D1912: Don't infer CallStacks.
  • elaborate the HasCallStack docs with an example of HasCallStack inferrence
  • use any in the definition of elemType
Mar 21 2016, 5:49 PM · GHC
gridaphobe updated the Trac tickets for D1912: Don't infer CallStacks.
Mar 21 2016, 5:18 PM · GHC
gridaphobe updated the diff for D1912: Don't infer CallStacks.

fix T11145

Mar 21 2016, 4:48 PM · GHC
gridaphobe updated the diff for D1912: Don't infer CallStacks.

break import loop

Mar 21 2016, 4:46 PM · GHC
gridaphobe updated the diff for D1912: Don't infer CallStacks.

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

Mar 21 2016, 3:36 PM · GHC
gridaphobe updated the diff for D1912: Don't infer CallStacks.

add TopLevelFlag.GhciTopLevel and merge master

Mar 21 2016, 2:59 PM · GHC

Mar 7 2016

gridaphobe added inline comments to D1912: Don't infer CallStacks.
Mar 7 2016, 8:49 PM · GHC

Feb 19 2016

gridaphobe added inline comments to D1912: Don't infer CallStacks.
Feb 19 2016, 11:12 PM · GHC
gridaphobe updated the diff for D1912: Don't infer CallStacks.
  • 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 19 2016, 11:05 PM · GHC

Feb 15 2016

gridaphobe added a comment to D1912: Don't infer CallStacks.

Ah yes, I see. Well, try this: quantify over a Wanted call stack iff (a) the binding is not top level, or (b) the user specified HasCallStack in any of the sigs for this inferred binding.

Feb 15 2016, 10:34 AM · GHC
gridaphobe abandoned D1911: Print `?callStack::CallStack` as `HasCallStack`.

Since D1912 is based on this revision, I'm going to abandon this and work the changes into D1912 instead, thanks for the comments!

Feb 15 2016, 10:23 AM · GHC
gridaphobe added a comment to D1912: Don't infer CallStacks.
  • I suggest you ignore the signatures for a mutually recursive group. Simply don't quantify over call stacks when inferring, period.
Feb 15 2016, 10:18 AM · GHC

Feb 14 2016

gridaphobe updated the diff for D1911: Print `?callStack::CallStack` as `HasCallStack`.

good point, we might as well group the keys

Feb 14 2016, 9:46 PM · GHC
gridaphobe added a child revision for D1911: Print `?callStack::CallStack` as `HasCallStack`: D1912: Don't infer CallStacks.
Feb 14 2016, 12:09 AM · GHC
gridaphobe added a parent revision for D1912: Don't infer CallStacks: D1911: Print `?callStack::CallStack` as `HasCallStack`.
Feb 14 2016, 12:09 AM · GHC
gridaphobe added a comment to D1912: Don't infer CallStacks.

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 14 2016, 12:08 AM · GHC
gridaphobe retitled D1912: Don't infer CallStacks from to Don't infer CallStacks at the top-level.
Feb 14 2016, 12:05 AM · GHC

Feb 13 2016

gridaphobe retitled D1911: Print `?callStack::CallStack` as `HasCallStack` from to Print `?callStack::CallStack` as `HasCallStack`.
Feb 13 2016, 4:12 PM · GHC

Feb 11 2016

gridaphobe accepted D1903: Add IsList instance for CallStack, restore Show instance for CallStack.

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 11 2016, 5:10 PM

Feb 9 2016

gridaphobe added a comment to D1894: Restore missing derived instances for CallStack and SrcLoc.

Ok then let's just print it as a list. We could add an IsList instance too, that seems reasonable.

Feb 9 2016, 7:26 PM
gridaphobe added a comment to D1894: Restore missing derived instances for CallStack and SrcLoc.

Right, that's why we expose pushCallStack instead of the constructor :)

Feb 9 2016, 1:12 PM
gridaphobe added a comment to D1894: Restore missing derived instances for CallStack and SrcLoc.

Oops, sorry for charging forth without seeking your advice!

No worries!

Feb 9 2016, 12:41 PM
gridaphobe added a comment to D1894: Restore missing derived instances for CallStack and SrcLoc.

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 9 2016, 10:05 AM

Feb 8 2016

gridaphobe accepted D1894: Restore missing derived instances for CallStack and SrcLoc.

Thanks!

Feb 8 2016, 8:52 PM

Feb 5 2016

gridaphobe retitled D1886: Add a derived `Show SrcLoc` instance from to Add a derived `Show SrcLoc` instance.
Feb 5 2016, 11:18 AM · GHC

Jan 27 2016

gridaphobe updated the diff for D1818: Hide the CallStack implicit parameter.

give the HasCallStack alias in the Note, add the API warning to the user's guide as well

Jan 27 2016, 6:07 PM · GHC
gridaphobe updated the diff for D1818: Hide the CallStack implicit parameter.

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).

Jan 27 2016, 4:16 PM · GHC
gridaphobe added a comment to D1818: Hide the CallStack implicit parameter.

@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 27 2016, 10:59 AM · GHC

Jan 26 2016

gridaphobe updated D1818: Hide the CallStack implicit parameter.
Jan 26 2016, 12:13 PM · GHC
gridaphobe updated the diff for D1818: Hide the CallStack implicit parameter.

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 26 2016, 12:13 PM · GHC

Jan 25 2016

gridaphobe added inline comments to D1818: Hide the CallStack implicit parameter.
Jan 25 2016, 6:06 PM · GHC
gridaphobe updated the diff for D1818: Hide the CallStack implicit parameter.

tweak code blocks

Jan 25 2016, 6:05 PM · GHC
gridaphobe updated the diff for D1818: Hide the CallStack implicit parameter.

A first draft of the revised CallStack docs, comments are very welcome.

Jan 25 2016, 1:05 PM · GHC

Jan 24 2016

gridaphobe added a comment to D1798: Fix a formatting error in the user's guide.

Thanks, I'm rewriting this section anyway as part of D1818, so I'll fix the typos there.

Jan 24 2016, 1:55 PM · GHC

Jan 21 2016

gridaphobe updated the diff for D1804: Default non-canonical CallStack constraints.

@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 :)

Jan 21 2016, 4:36 PM · GHC
gridaphobe retitled D1818: Hide the CallStack implicit parameter from to Hide the CallStack implicit parameter.
Jan 21 2016, 3:47 PM · GHC
gridaphobe added a comment to D1805: Change runtime linker to perform lazy loading of symbols/sections.
In D1805#53370, @thomie wrote:

validate --fast passes for me on OSX 10.11.2

You'll have to set DYNAMIC_GHC_PROGRAMS = NO in mk/validate.mk (that you create yourself), for this to be an effective test. By default ghc uses the system linker on Mac.

Jan 21 2016, 12:24 PM
gridaphobe added a comment to D1804: Default non-canonical CallStack constraints.

@bgamari this will need to be applied to 8.0 as well, fyi.

Jan 21 2016, 11:13 AM · GHC

Jan 20 2016

gridaphobe added a comment to D1805: Change runtime linker to perform lazy loading of symbols/sections.
In D1805#53321, @Phyx wrote:

I guess all I'm missing now is a Mac OS validate.

Jan 20 2016, 6:43 PM
gridaphobe updated the diff for D1804: Default non-canonical CallStack constraints.
isCallStackCt :: Ct -> Maybe FastString
Jan 20 2016, 11:50 AM · GHC

Jan 19 2016

gridaphobe added a comment to D1804: Default non-canonical CallStack constraints.

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.

Jan 19 2016, 7:57 PM · GHC
gridaphobe updated the diff for D1804: Default non-canonical CallStack constraints.

clean up the test case

Jan 19 2016, 7:10 PM · GHC
gridaphobe added inline comments to D1804: Default non-canonical CallStack constraints.
Jan 19 2016, 6:55 PM · GHC
gridaphobe added inline comments to D1804: Default non-canonical CallStack constraints.
Jan 19 2016, 6:47 PM · GHC
gridaphobe added a comment to D1804: Default non-canonical CallStack constraints.

@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.

Jan 19 2016, 6:03 PM · GHC
gridaphobe retitled D1804: Default non-canonical CallStack constraints from to Default non-canonical CallStack constraints.
Jan 19 2016, 6:01 PM · GHC

Jan 18 2016

gridaphobe added a comment to D1798: Fix a formatting error in the user's guide.

I believe this patch also needs to be applied to the 8.0 branch.

Jan 18 2016, 4:39 PM · GHC