simonmar (Simon Marlow)
User

Projects

User Details

User Since
Jun 8 2014, 1:59 AM (267 w, 2 d)

Recent Activity

Apr 2 2019

Marge Bot <ben+marge-bot@smart-cactus.org> committed rGHCbf73419518ca: Add myself to libraries/ghci (authored by simonmar).
Add myself to libraries/ghci
Apr 2 2019, 3:54 PM

Jan 23 2019

simonmar updated subscribers of D4263: [RTS] Add loadNativeObj and unloadNativeObj.

The status is that we still need this at FB and we're maintaining it as a local patch for now. It needs some work to be pushed upstream (see above discussion) which we really should do...

Jan 23 2019, 2:36 AM

Jan 21 2019

simonmar accepted D5433: Introduce GhciMonad and generalize types of functions in GHCi.UI.
Changing the type of a function from GhcMonad g => g a to GHCi a or from GHCi a to InputT GHCi a will also more likely result in a change in callsite.
Jan 21 2019, 3:42 AM

Dec 21 2018

simonmar added a comment to D5432: Do Caf analysis right before codegen, fix #9718.

We can grep for idCafInfo and see there are none!

Dec 21 2018, 8:56 AM
simonmar added a comment to D5460: Deactivate stack squeezing for `cacheprof`.

Ah OK. Commented on Trac #16065.

Dec 21 2018, 4:16 AM
simonmar added a comment to D5432: Do Caf analysis right before codegen, fix #9718.

What is making me uncomfortable is that our current design looks like this:

  • We get the Name -> CafInfo map
  • We run over the top level binding, augmenting the CafInfo of the _binding sites_ of top level Ids. We don't modify the _occurrences_. Apart from the tiresome inefficiency, it offends me that the binding sites and the occurrence sites differ, in an important way.
  • When code-generating, at the binding site of a let (or at top level) we make a CgIdInfo for each binding, which contains the Id from the binding site
  • At each _occurrence_ site we look up the CgIdInfo for that Id -- and we must be super-careful to use the Id from the CgIdInfo, and NOT the one at the occurrence site, because only the former has correct CafInfo.

    What to do? It's not impossible as-is, provided it is very carefully documented. As a runtime check, I suppose we could make the CafInfo for an Id be a Maybe CafInfo with Nothing meaning "you should not be looking at me". (But that costs us at runtime.)
Dec 21 2018, 4:12 AM
simonmar added a comment to D5433: Introduce GhciMonad and generalize types of functions in GHCi.UI.

What is it that forces the overloading?

Right now, nothing, we can always change GhciMonad m => m a to GHCi a and use lift when necessary.
It may help prevent GHCi a being changed to InputT GHCi a unnecessarily.

Dec 21 2018, 2:36 AM
simonmar added a comment to D5460: Deactivate stack squeezing for `cacheprof`.

I note that Trac #4450 had exactly the same diagnosis that we've just rediscovered for Trac #8611, 8 years ago. So to avoid this happening again for more benchmarks, why don't we just do +RTS -V0 for all the single-threaded benchmarks in nofib? That is, add it by default to all parts of nofib except nofib/smp and nofib/parallel.

Dec 21 2018, 2:20 AM
simonmar added a comment to D5432: Do Caf analysis right before codegen, fix #9718.
In D5432#150837, @osa1 wrote:

@simonpj one thing I realized is because we don't generate Stg when generating bytecode we can't do CafInfo analysis for interpreted modules, so we can't update ModDetails/HomeModInfo of interpreted modules with CafInfos. Do you think this is a problem?

Dec 21 2018, 2:12 AM
simonmar added a comment to D5432: Do Caf analysis right before codegen, fix #9718.

Remove CafInfo from IdLabel entirely.
Pass the Name -> CafInfo mapping directly to CmmBuildnfoTables; when you would do hasCaf lbl instead do hasCaf caf_env lbl:

Dec 21 2018, 2:08 AM

Dec 18 2018

simonmar requested changes to D5432: Do Caf analysis right before codegen, fix #9718.
In D5432#150596, @osa1 wrote:

As far as I can tell, in this diff we calculate the fingerprints as normal in mkIface but then update the CafInfo later. Isn't this wrong? The fingerprints need to take into account the CafInfo, so we have to defer computing the fingerprints until after the CafInfo is added.

Currently we generate interface files only once so this should be fixed. (in the revision you reviewed I was generating interfaces with conservative CafInfos and then updating with precise CafInfos, without updating fingerprints)

Dec 18 2018, 9:08 AM

Dec 14 2018

simonmar added a comment to D5432: Do Caf analysis right before codegen, fix #9718.

As far as I can tell, in this diff we calculate the fingerprints as normal in mkIface but then update the CafInfo later. Isn't this wrong? The fingerprints need to take into account the CafInfo, so we have to defer computing the fingerprints until after the CafInfo is added.

Dec 14 2018, 12:34 PM
simonmar added a comment to D5433: Introduce GhciMonad and generalize types of functions in GHCi.UI.

Oh, and in general I think InputT has infested much more of the GHCi code than it really should, it would be nice to restrict it to a much smaller area.

Dec 14 2018, 2:54 AM
simonmar added a comment to D5433: Introduce GhciMonad and generalize types of functions in GHCi.UI.

I'd like to understand a bit more about the plan here. In general it's not necessary to overload everything in order to use the functions in a different monad - we can lift the functions instead. What is it that forces the overloading? Perhaps it's something to do with the structure of the interpreter, if we want to add more commands then the interpreter is calling back into our code, so we need to allow a different monad to be threaded through?

Dec 14 2018, 2:53 AM
simonmar accepted D5453: Handle :cd in external interpreter in a more robust way.

Yes, that's better

Dec 14 2018, 2:35 AM
simonmar accepted D5452: Fix ghci crash when starting with -fno-implicit-import-qualified.

LGTM

Dec 14 2018, 2:32 AM

Dec 13 2018

simonmar added a comment to D5445: Update -F RTS help:.

Thanks!

Dec 13 2018, 4:11 AM

Dec 10 2018

simonmar added a comment to D5394: Fix recompilation bug with default class methods (#15970).

Actually I realised the Name is needed when fingerprinting so we couldn't make do with just the OccName, but I added a Note anyway.

Dec 10 2018, 2:34 AM
simonmar updated the diff for D5394: Fix recompilation bug with default class methods (#15970).

add Note

Dec 10 2018, 2:32 AM
simonmar created D5428: Add +RTS -F to the --help output.
Dec 10 2018, 2:11 AM

Dec 7 2018

simonmar added a comment to D5414: Don't use a generic apply thunk for known calls.

Yet another place where we want a way for the user to say whether they want to tune for size or speed, like gcc's -Os flag.

Dec 7 2018, 2:37 AM
simonmar accepted D5421: Mark SRT_1/SRT_2 as CONSTR_1_0/CONSTR_2_0.

This makes zero difference to anything, because we land in the same switch branch: https://phabricator.haskell.org/diffusion/GHC/browse/master/rts%2Fsm%2FScav.c$1747-1753

Dec 7 2018, 2:31 AM

Dec 5 2018

simonmar added inline comments to D5394: Fix recompilation bug with default class methods (#15970).
Dec 5 2018, 3:30 PM
simonmar added inline comments to D5394: Fix recompilation bug with default class methods (#15970).
Dec 5 2018, 3:26 PM
simonmar accepted D5407: cosmetic change: expandtab in utils/hp2ps/HpFile.c.

Yay. This code is so old!

Dec 5 2018, 2:51 AM

Nov 30 2018

simonmar added a comment to D5392: Deduplicate decision to count thunks in `-ticky`.
In D5392#148664, @sgraf wrote:

I think I would probably avoid the Bool argument by having a separate dynThunkHasCounter, but up to you.

What would that look like? Wouldn't that be just tickyDynThunkIsOn? I agree that Bool has probably little documentary value. Maybe just add data Lifetime = Static | Dynamic and use that?

Nov 30 2018, 4:41 AM
simonmar added a comment to D4780: a naive but simple fix to resolve trac ticket 15207, because currently stgcrun.c when compiled with gcc and then assembled with clang/apple as/llvm assembler fails..

So neither fix is committable in its current state, if I'm understanding correctly? How would you like to move forward with this, do you need some help?

Nov 30 2018, 2:26 AM
simonmar accepted D5392: Deduplicate decision to count thunks in `-ticky`.

I think I would probably avoid the Bool argument by having a separate dynThunkHasCounter, but up to you.

Nov 30 2018, 2:24 AM
simonmar requested changes to D4110: Introduce with# as preferred alternative to touch#.

This doesn't need a new WITH_FRAME type in the RTS, because we never need to treat these stack frames specially when walking the stack. There are a few examples of primitives that create new stack frames without adding a new frame types, e.g. maskAsyncExceptionszh_ret.

Nov 30 2018, 2:14 AM

Nov 29 2018

simonmar accepted D5384: Rip out object splitting.

intothefire

Nov 29 2018, 9:57 AM
simonmar created Image Macro "intothefire".
Nov 29 2018, 9:54 AM
simonmar added reviewers for D5394: Fix recompilation bug with default class methods (#15970): watashi, afarmer.
Nov 29 2018, 7:09 AM
simonmar updated the test plan for D5394: Fix recompilation bug with default class methods (#15970).
Nov 29 2018, 7:06 AM
simonmar created D5394: Fix recompilation bug with default class methods (#15970).
Nov 29 2018, 7:06 AM

Nov 26 2018

simonmar added a comment to D5358: Remove redundant check in cgCase.

@osa1 did it get reverted?

Nov 26 2018, 2:51 AM
simonmar planned changes to D5204: EXPERIMENTAL: different codegen for tag-check on entering (#8905).

Yes, good catch. It seems that we need something more refined than we have here.

Nov 26 2018, 2:50 AM

Nov 23 2018

simonmar added a comment to D5224: Implement late lambda lift.

Do you have the nofib numbers for compilation time and allocations?

Nov 23 2018, 5:14 AM
simonmar added a comment to D5224: Implement late lambda lift.

What impact does this have on compile time? I notice that you've enabled it at both -O and -O2.

Nov 23 2018, 2:19 AM
simonmar accepted D5369: RetainerProfiler: Update retainer profiler debugging.

I'm really happy to see the retainer profiler getting some love. Thanks!

Nov 23 2018, 2:12 AM

Nov 22 2018

simonmar added a comment to D5358: Remove redundant check in cgCase.

CmmSink is only run with -O, so perhaps we lose a little performance with -O0? You might argue that we don't care about that, but the goal of -O0 is to get as good code as possible with little effort. If we have dead binder info (even if it's partial) then we should use it.

Nov 22 2018, 5:57 AM
simonmar accepted D5319: Remove warnings-silencing flags for code generated by Alex.

Yep, LGTM

Nov 22 2018, 5:53 AM
simonmar accepted D5363: rts: fix Windows megablock allocator.

Good catch.

Nov 22 2018, 5:51 AM

Nov 21 2018

simonmar added inline comments to D5342: Fix heap corruption during stable name allocation.
Nov 21 2018, 10:13 AM

Nov 19 2018

simonmar added a comment to D5342: Fix heap corruption during stable name allocation.

The issue is that if a thread goes into a tight loop doing nothing but calls to makeStableName#, then without a call to MAYBE_GC() we will fill up the memory and never GC.

Nov 19 2018, 3:03 AM
simonmar added a comment to D5005: Remove completed threads from the gen->threads list eagerly.

Oh, I'd completely forgotten about this patch. I'll see if we can get some numbers.

Nov 19 2018, 3:00 AM
simonmar accepted D5346: Fix uninformative hp2ps error when the command args contains double quotes.

Seems reaosnable.

Nov 19 2018, 2:58 AM
simonmar accepted D5349: Fix deadlock bug when mkFastStringWith is duplicated.

I think this is a general problem with unsafeDupablePerformIO that we can't do bracket-type things inside it. I'm thinking about possible solutions.

Nov 19 2018, 2:55 AM
simonmar added a comment to D5342: Fix heap corruption during stable name allocation.

Oh, you also need to add a MAYBE_GC() call to the function.

Nov 19 2018, 2:46 AM
simonmar accepted D5342: Fix heap corruption during stable name allocation.

Much simpler! allocate() is fine, it's used by lots of primops - in particular the Integer primops that need to allocate memory inside GMP use it, because we can't GC there either.

Nov 19 2018, 2:46 AM
simonmar added inline comments to D5353: linker: store entire link map and use it..
Nov 19 2018, 2:43 AM
simonmar accepted D5289: Add a RTS option -xp to load PIC object anywhere in address space.

Let's do it.

Nov 19 2018, 2:30 AM

Nov 17 2018

simonmar requested changes to D5342: Fix heap corruption during stable name allocation.

Good catch. But I think the fix is much too complicated - we could just call allocate() instead of ALLOC_PRIM(), which never fails. We'd also need a MAYBE_GC call at the beginning of the function.

Nov 17 2018, 6:13 AM

Nov 16 2018

simonmar accepted D5333: More efficient, non-allocating unsafeLookupStaticPtr.

LGTM

Nov 16 2018, 2:11 AM
simonmar accepted D5319: Remove warnings-silencing flags for code generated by Alex.
Nov 16 2018, 2:10 AM
simonmar added a comment to D5280: Fix #15758 by making the RTS parse response file arguments.

Should getArgsWithResponseFiles still remove the RTS options that it encounters while expanding response files?
Sure, the options won't have any effect on the RTS configuration itself, but might save some of the users from having to do that work.

Nov 16 2018, 2:06 AM

Nov 15 2018

simonmar accepted D5337: Some assertions and comments in scheduler.

Yep, all looks sensible to me.

Nov 15 2018, 5:39 AM

Nov 14 2018

simonmar created D5336: Use --no-as-needed with LLD too.
Nov 14 2018, 8:47 AM
simonmar created D5334: Fix a bug in SRT generation (#15892).
Nov 14 2018, 8:33 AM
simonmar accepted D5325: rts: Ensure that task->id is initialized.

Doesn't this get set by the caller of newTask()?

Nov 14 2018, 2:12 AM

Nov 12 2018

simonmar added a comment to D5280: Fix #15758 by making the RTS parse response file arguments.

In this patch, I have copied the relevant bits from the GCC source code, and modified it to fit the GHC conventions.
But, we must make sure that we don't violate the GPL.

Nov 12 2018, 2:29 AM

Nov 9 2018

simonmar added a comment to D4780: a naive but simple fix to resolve trac ticket 15207, because currently stgcrun.c when compiled with gcc and then assembled with clang/apple as/llvm assembler fails..

If there's a better fix, please abandon this diff.

Nov 9 2018, 2:30 AM
simonmar accepted D5287: eventlog: Log the current stack size when stack overflows.
Nov 9 2018, 2:27 AM
simonmar accepted D5306: UNREG: PprC: Add support for adjacent floats.

I think we'll need to do something much more general than this in the future when we have a mixture of Int8#, Int16#, etc. fields that need to be packed into words. Happy to go with this for now though.

Nov 9 2018, 2:10 AM

Nov 5 2018

simonmar accepted D5288: Explicitly pass -fno-PIC to C compiler on linux.

Trac Trac #15847: We don't support loading non-PIC .o on i386, this is why I noticed this issue

Nov 5 2018, 2:16 AM
simonmar requested changes to D5287: eventlog: Log the current stack size when stack overflows.
Nov 5 2018, 2:09 AM

Nov 2 2018

simonmar added a comment to D5288: Explicitly pass -fno-PIC to C compiler on linux.

I'm not sure I understand the problem here. Why is there a missing symbol error?

Nov 2 2018, 3:38 AM
simonmar accepted D5290: Allocate bss section within proper range of other sections.
Nov 2 2018, 3:32 AM
simonmar accepted D5293: rts: Allow output filename of eventlog to be specified on the command line.

Sure, seems reasonable.

Nov 2 2018, 3:28 AM
simonmar accepted D5292: rts: Add FALLTHROUGH macro.

Fine by me

Nov 2 2018, 3:15 AM

Oct 26 2018

simonmar added a comment to D5211: Rewrite FastString table in concurrent hashtable.

Thanks for adding the test!

Oct 26 2018, 2:24 AM
simonmar added inline comments to D5258: Roll forward "Add Int8# and Word8#".
Oct 26 2018, 2:20 AM

Oct 24 2018

simonmar updated subscribers of D5255: hadrian: build ghc-iserv-prof in addition to ghc-iserv, as it is required for 10+ tests.
Oct 24 2018, 10:28 AM
simonmar added inline comments to D5255: hadrian: build ghc-iserv-prof in addition to ghc-iserv, as it is required for 10+ tests.
Oct 24 2018, 9:44 AM
simonmar accepted D5211: Rewrite FastString table in concurrent hashtable.

This is great. It could be even better if

  • We had your benchmark in the test suite, perhaps testsuite/tests/perf? We can use small parameters so it doesn't run for long.
  • We point to the benchmark from comments in the code, so that others can use it when making future changes.
Oct 24 2018, 8:27 AM

Oct 19 2018

simonmar added a comment to D4726: NCG: New code layout algorithm..

A bit more review

Oct 19 2018, 2:42 AM
simonmar added a comment to D5244: includes: Allow headers to be built with C++11 compilers.

Will this work with g++ with no flags? If not, what else would be needed?

Oct 19 2018, 2:23 AM

Oct 18 2018

simonmar accepted D5233: Don't use X86_64_ELF_NONPIC_HACK for +RTS -xp.

btw, use #1234 for ticket references in Phabricator markup.

Oct 18 2018, 1:22 PM
simonmar accepted D5234: Fix ghc-pkg when only prof way is enabled.

nicecatch

Oct 18 2018, 1:19 PM
simonmar added a comment to D5232: Remove StgBinderInfo and related computation in CoreToStg.

I think we could remove this, it's a very small code-size optimisation and would require a fair bit more work to implement, not at all clear that it's worth it. I commented more on the ticket.

Oct 18 2018, 1:15 PM

Oct 15 2018

simonmar added a comment to D5169: Merge sections in profiling .a to .p_o and use it whenever it exists.
  1. We should build these at installation time, so they don't have to go into the binary distribution (the same goes for the vanilla way` libHSfoo.o` files)
  2. This also needs to be done in Hadrian
Oct 15 2018, 11:43 AM

Oct 10 2018

simonmar accepted D5214: Generate correct relocation for external cost centre.

Thanks!

Oct 10 2018, 2:59 PM
simonmar accepted D5219: Allocate bss section within proper range of other sections.
Oct 10 2018, 2:59 PM
simonmar added a comment to D5219: Allocate bss section within proper range of other sections.

This looks OK.

Oct 10 2018, 2:59 PM

Oct 9 2018

simonmar added inline comments to D5214: Generate correct relocation for external cost centre.
Oct 9 2018, 3:15 PM
simonmar added a comment to D5210: Add pprList method to Outputable for better String rendering.

For Maybe [String] I would just pprDebug $ text (show x).

Oct 9 2018, 2:04 AM

Oct 8 2018

simonmar added a comment to D5211: Rewrite FastString table in concurrent hashtable.

Wow! Some questions below.

Oct 8 2018, 2:43 AM
simonmar added a comment to D5210: Add pprList method to Outputable for better String rendering.

It's not obvious to me that we should render a String like this in Outputable, after all we don't render a FastString with double quotes, and we don't render a Char with single quotes. Outputable is not intended to produce Haskell concrete syntax, it's mainly for pretty-printing the various internal compiler data structures.

Oct 8 2018, 2:18 AM
simonmar accepted D5212: ghc-heap: Fix writing closures on big endian.
Oct 8 2018, 2:11 AM
simonmar accepted D5201: Fix dataToTag# argument evaluation.
Oct 8 2018, 2:07 AM

Oct 7 2018

simonmar added a comment to D5201: Fix dataToTag# argument evaluation.

Looks good. Was that with the libraries recompiled too?

Oct 7 2018, 2:34 PM
simonmar added a comment to D5201: Fix dataToTag# argument evaluation.
In D5201#143481, @osa1 wrote:

I'd be really interested to see what effect this has on performance and code size too.

Why do you think there will be any difference? We already generate a case around the argument to evaluate it, but we used to do it in Core and sometimes avoid it (incorrectly). Now we do it in Cmm. So the code should really be the same.

I can still test of course, but I'm just curious why you think this makes a difference in code size or performance.

Oct 7 2018, 3:31 AM

Oct 6 2018

simonmar added a comment to D5201: Fix dataToTag# argument evaluation.

I'd be really interested to see what effect this has on performance and code size too.

Oct 6 2018, 4:02 PM
simonmar accepted D5207: UNREG: don't prefix asm prefixes in via-C mode.

Thanks!

Oct 6 2018, 7:05 AM
simonmar accepted D5209: Add a missing write barrier to small array writes.
Oct 6 2018, 7:03 AM

Oct 4 2018

simonmar added a comment to D5204: EXPERIMENTAL: different codegen for tag-check on entering (#8905).

nofib results below

Oct 4 2018, 11:21 AM
simonmar created D5204: EXPERIMENTAL: different codegen for tag-check on entering (#8905).
Oct 4 2018, 11:21 AM
simonmar added a comment to D4717: [WIP] CoreToStg: Try treating String unpackings as single-entry.

@osa1 We save the old info pointer when entering a CAF: https://phabricator.haskell.org/diffusion/GHC/browse/master/rts%2Fsm%2FStorage.c$406

Oct 4 2018, 2:29 AM
simonmar accepted D5195: Add a RTS option -xp to load PIC object anywhere in address space.

Looks good to go, provided it passes validate.

Oct 4 2018, 1:58 AM

Oct 3 2018

simonmar added inline comments to D5195: Add a RTS option -xp to load PIC object anywhere in address space.
Oct 3 2018, 2:46 AM

Oct 1 2018

simonmar added a comment to D4717: [WIP] CoreToStg: Try treating String unpackings as single-entry.

Another option might be to keep top-level strings updatable, but make the GC revert the CAF. That way we would get some of the sharing back.

Oct 1 2018, 2:34 PM