If I could, I'd give you extra points for the HHGTTG references.

Looks good to me!

• Tests for T14375
• Remove NOINLINE on allocaBytes... functions

Added tests.

This patch should have gone into D4110.

Manually rebased onto master

Not sure if these two are actually worth fixing, but thought I'd mention them anyway.

I only see the types defined, but not the actual accessors mentioned in the comment. Are you sure you have committed all changes?

@bgamari See Trac #14880; the roadmap is to get step 2 committed, and after that revisit the work done in this patch, however I believe that Richard and Simon might end up rethinking this part entirely once step 2 is committed.

• Fix some test wibbles

• Document TyCoRep changes better

• Remove commented-out code, turn it into Note text instead.

Rebase onto current master

Is there any test that reflects this change?

Hmm, the ticket talks about making this feature opt-in, which makes a lot of sense to me, but this patch just drops the feature entirely. What's the reasoning behind this?

Assembled a Note from various snippets from the Trac issue and surrounding discussion. Feel free to edit rigorously.

• Add Note [Closing over free variable kinds]

• Add Note [Closing over free variable kinds]

In D5147#141553, @simonpj wrote:

We'll need a big Note about all this

Fixup

Use extendVarSet, not unionFV

In D5147#141559, @simonpj wrote:

In D5147#141557, @tdammers wrote:

Use Interesting Function

But you did not take advantage of my suggestion

| otherwise = tyCoFVsOfType (tyVarKind v)) f emptyVarSet (v:acc_list, extendVarSet acc_set v)

unitFV will just duplicate three tests you have already made!

Use Interesting Function

• Bugfix

OK, so I added test3 from the ticket, which probably reflects the problem most accurately. I don't think adding more of the test cases would add much.

• Add Notes for Trac #15578
• Fix test for Trac #15578
• Add regression test for Trac #15578

• Add regression test for Trac #15578

I suggest continuing the discussion on Trac 15363 rather than here, if only to preserve it for future generations.

That build failure is strange. Apparently enabling -O2 on stage1 triggers a bug that doesn't surface on Linux. gcc vs. clang, possibly?

Ideally I'd like to see this tested on more machines and platforms, but if those timings are anywhere near accurate, I'd expect those to match too.

These look useful, but I'm not sure how reluctant we are to add more things to base just like that.

I agree that this should be properly solved eventually. Workaround looks fine though.

Could you maybe add a test or two? You already have an example program that would highlight a regression.

I have trouble understanding what this would buy us. 452 lines of Windows-specific code are a liability (most GHC devs are not on Windows, and aren't keen on maintaining nontrivial Windows code), so the benefit would have to be substantial to make it worthwhile.

I've looked a bit deeper, and it turns out that the "bug" isn't really a bug - essentially what bytestring is doing here is re-exporting findSubstring from Data.ByteString from Data.ByteString.Char8. More details in the ticket (Trac #15337).

Well, here's the relevant bit from the build log:

In D4915#135418, @bgamari wrote:

Yes, I agree that this is the right way forward. This also means that we can simply remove split-objs, since Windows is AFAIK the only platform that still used it.

Looks like the build fails because it can't find some .o files. Might want to look into that.

Maybe check out how this relates to D4929? There doesn't seem to be any overlap between the two patches, but D4929 puts foldl' in GHC.Prelude, which make the Data.List import redundant here.

Would it be possible to see the performance improvements reflected in the test suite somewhere? E.g., by adding a test that compiles a program that would benefit a lot from this in terms of compilation time?

• Please fix the parser error / build failure
• A more descriptive commit message would be great

Apart from the stylistic remarks; I wonder whether this change would pull its weight. The Mach- prefix doesn't seem entirely outrageous to me (after all, these are things that are supposed to be "close to the metal" or "in machine format"), and changing them has a bit of an impact wrt future patches, merging, etc. But I really can't judge which way the balance would tip.

What about those failing CI tests? I can't make perfect sense of it, but IIUC, one test now allocates 16% less (which is to be expected, I think?), while another test now fails.

I think this deserves a proper discussion in a Trac issue, with some background information and concrete motivating examples.

In D4820#133110, @simonpj wrote:

Thanks for doing this!

The lack of INLINE breaks stream fusion, leading to degeneration in performance for Int32, Int64 and similar types

Do you understand *why* the lack of INLINE breaks fusion. It's be really good to give a little example to illustrate the steps, and where they break. Otherwise it's too much like alchemy... "try it and see".

Also could you add this info in a Note? Otherwise someone in 5 years time might wonder what those INLINE pragmas are doing, and remove them. (Yes, if they are careful enough they could do a git-annotate, but it's not so clear.)

In D4827#133290, @Ninjatrappeur wrote:

I am a bit puzzled by the CI failure. I don't get why the runner cannot run ./validate.

Did I mess something up here?

Is there some ticket associated with this? Do we know the context in which it allocated too much? *Someone* has clearly run into this in the past, and before undoing the "fix", I'd like some assertion that the problem has actually gone away, rather than us just not running it in the circumstances that trigger it.

Fantastic, thanks a lot.

Abandoning in favor of D4830.

This solves the same problem as D4833, but seems like a more elegant solution, so I suggest we roll with this patch instead of mine.

In D4833#133260, @sighingnow wrote:

@tdammers We have already disabled -fdefer-type-errors and -fdefer-typed-holes in GHCi (by Phab:D1527), see https://github.com/ghc/ghc/blob/master/compiler/typecheck/TcRnDriver.hs#L2070. I opened a ticket Trac #15259 yesterday to disable -fdefer-out-of-scope-variables as well, and fixed it by Phab:D4830. After doing that, the program given in Trac #14963 prints the result correctly and doesn't panic.

In D4833#133276, @sighingnow wrote:

I think it's ok to allow -fdefer-type-errors in files to be load to ghci, which wouldn't cause a panic and is very helpful to develop and try partial finished programs. We only need to disable -fdefer-type-errors (as well as the other two -fdefer) to interactive inputs in ghci.

• Disable error deferring in interactive statements
• Documentation
• Test for Trac #14963 workaround
• Updated test output

• Override interactive dynflags, not session
• Updated test output

Your test is failing on OS X; seems to be a difference in how the wc implementations format their output - the Linux one left-aligns the 0, while the OS X one pads it with spaces. A not very elegant, but effective, solution would be to throw a quick sed job into the pipeling, something like | sed -e 's/^\s*//' maybe?

Might be confusing for an unsuspecting reader to read hints about swapping the branches of a conditional, and then seeing code that also reduces conditionals down to just the plain branches.

Updated to reflect changed output in tests, and adding -Werror to tests that should fail.

Marking as "request changes", because this is clearly WIP.