Kill the splitter with fire
Needs RevisionPublic

Authored by DemiMarie on Nov 29 2016, 10:06 PM.

Details

Summary

Kill the splitter and all associated code.

The splitter is an evil Perl script that processes assembler code.
Its job can be done better by the linker's --gc-sections flag. GHC
passes this flag to the linker whenever -split-sections is passed on
the command line.

Fixes Trac #11315
Fixes Trac #9832
Fixes Trac #8964
Fixes Trac #8685
Fixes Trac #8629

Test Plan

GHC CI

There are a very large number of changes, so older changes are hidden. Show Older Changes

Fix silly type error in unregisterized mode

DemiMarie marked 3 inline comments as done.Dec 1 2016, 7:48 AM
Phyx requested changes to this revision.Dec 1 2016, 8:22 AM
Phyx added a reviewer: Phyx.
Phyx added a subscriber: Phyx.

As I've mentioned multiple times before. The -split-sections return invalid PE code in Windows. This Cannot go in without a working -split-sections.

We should not break a working feature with something that does not work.

This revision now requires changes to proceed.Dec 1 2016, 8:22 AM
Phyx edited edge metadata.Dec 1 2016, 8:22 AM
Phyx removed a reviewer: Phyx.
Phyx removed a subscriber: Phyx.
This revision is now accepted and ready to land.Dec 1 2016, 8:23 AM
simonmar requested changes to this revision.Dec 1 2016, 8:27 AM
simonmar edited edge metadata.

Fix Windows first

This revision now requires changes to proceed.Dec 1 2016, 8:27 AM
DemiMarie updated this revision to Diff 9716.Dec 1 2016, 8:38 AM
DemiMarie edited edge metadata.

Fix unused imports warning

DemiMarie updated this revision to Diff 9723.Dec 1 2016, 10:01 AM
DemiMarie edited edge metadata.

Fix a compilation error

due to -Werror and an unused variable

bgamari edited edge metadata.Dec 1 2016, 11:23 AM

Great work @DemiMarie! A few comments inline.

compiler/main/DriverPipeline.hs
1205

Kill the "not split" parenthetical.

2110

Why is it necessary to force dflags here?

compiler/main/SysTools.hs
277

s/utilitie/utility/

compiler/main/TidyPgm.hs
1035–1036

If I'm not mistaken: We don't externalise names with split sections, so the comment can certainly go. However, note the comment regarding the BCO symbol table below. In principle we only need to rename here if we are targeting bytecode. That being said, we would need to have DynFlags to determine this.

I wonder whether it is worth the hassle of conditionally renaming. It would be nice to characterize the effect of this on compiler performance.

utils/haddock
1

Is this change necessary? If so where can the relevant branch be found?

DemiMarie updated this revision to Diff 9741.Dec 1 2016, 2:43 PM
DemiMarie edited edge metadata.

Fix submodule

DemiMarie updated this revision to Diff 9743.Dec 1 2016, 3:15 PM
DemiMarie edited edge metadata.

Don't externalize names when not needed; fix comments.

DemiMarie marked 6 inline comments as done.Dec 1 2016, 3:22 PM
DemiMarie added inline comments.
compiler/main/DriverPipeline.hs
2110

It isn't; I included this to avoid a warning. I can either kill the DynFlags argument outright or I can simply ignore it.

DemiMarie marked an inline comment as done.Dec 1 2016, 3:22 PM
DemiMarie updated this revision to Diff 9744.Dec 1 2016, 3:34 PM
DemiMarie edited edge metadata.

Fix silly type error

@bgamari Haddock doesn't build without a patch (PR #562 on GitHub).

DemiMarie updated this revision to Diff 9780.Dec 3 2016, 12:01 PM
DemiMarie edited edge metadata.

Bring back Opt_SplitObjs so Haddock builds

bgamari requested changes to this revision.Dec 6 2016, 12:13 AM
bgamari edited edge metadata.

Great! Looks like you are almost there; you just need to update the testsuite output and add a reference in the release notes (docs/users_guide/8.2.1-relnotes.rst) and I think we're all set.

This revision now requires changes to proceed.Dec 6 2016, 12:13 AM

Is Windows fixed?

Is Windows fixed?

The pieces are in place but there is a little bit more work necessary to make it work out of the box. See Trac #11445.

DemiMarie updated this revision to Diff 9809.Dec 6 2016, 12:44 PM
DemiMarie edited edge metadata.

Fix stderr output in testsuite

to account for GHC no longer externalizing names when not needed

DemiMarie updated this revision to Diff 9810.Dec 6 2016, 12:47 PM
DemiMarie edited edge metadata.

Update User's Guide

bgamari added inline comments.Dec 8 2016, 3:54 PM
mk/config.mk.in
329–336

We should probably mention Trac #11445 here.

DemiMarie updated this revision to Diff 10165.Dec 22 2016, 11:33 PM
DemiMarie edited edge metadata.

Rebase

DemiMarie marked an inline comment as done.Dec 22 2016, 11:46 PM
DemiMarie updated this revision to Diff 10577.Jan 20 2017, 11:33 PM
DemiMarie edited edge metadata.

Rebase

DemiMarie updated this revision to Diff 10581.Jan 21 2017, 10:11 AM
DemiMarie edited edge metadata.

Fix compilation error

bgamari requested changes to this revision.Jan 21 2017, 12:28 PM
bgamari edited edge metadata.

@DemiMarie, it looks like you need to drop the (probably inadvertent) change to utils/haddock.

This revision now requires changes to proceed.Jan 21 2017, 12:28 PM
DemiMarie updated this revision to Diff 10586.Jan 21 2017, 12:55 PM
DemiMarie edited edge metadata.

Fix haddock submodule

If I recall correctly we are still waiting for split sections to work on Windows. We should likely hold off on merging this until this is fixed, so this is likely 8.4 material. Bumping out of review queue until then.

@DemiMarie, do correct me if I'm wrong.

bgamari requested changes to this revision.Feb 8 2017, 1:41 PM
This revision now requires changes to proceed.Feb 8 2017, 1:41 PM
bgamari added a subscriber: Phyx.Oct 11 2017, 11:37 AM

@Phyx, are we in a position yet where we can move forward on this?

Phyx added a comment.Oct 11 2017, 2:13 PM

@Phyx, are we in a position yet where we can move forward on this?

Unfortunately no, we do have working split-sections on x86_64 Windows, but not x86 due to ld lacking support for it on i386.
But even on x86_64 due to bfd being so slow, the link time is too long (if you remember the major delay in CI when we had it on.).

Realistically, should we want this, we need to switch linkers. Which is not easy because we use GCC as the link driver, so GCC has to
support whatever we switch to. This knocks out the Microsoft linker and lld. (lld on is always based on the most popular native compiler for
the platform, so on Windows it is made to support compatibility with cl and not ld.).

Which means, we need GCC patches to call lld (done), and we need lld to support a GNU style interface on Windows, (in progress). But there's no way
I'll make this for 8.4

Is there a way we can quit using GCC on Windows and just call the linker
directly?

Phyx added a comment.Oct 15 2017, 5:48 AM

Is there a way we can quit using GCC on Windows and just call the linker
directly?

This is a much harder thing to do. The linker flags are not being generated In just one place so supporting another linker interface would
either create a very intrusive mess or require a major refactoring.

That aside, the reason we use GCC is because we simply don't know how to link for the final executable. We don't know the implicit
dependencies GCC adds or where to find them. We don't understand the multi-lib or specs files concept either, so we can't currently
tell which crt to use. Basically any flag that affects ABI would be a potential issue, and we'd need to stop using linker scripts. We also
need to change/replace special symbols that ld would have normally provided, like the image base, etc.

In short yes it's doable, but I think it'll be much more work, fragile and the maintenance burden much higher.

In D2768#114417, @Phyx wrote:

Is there a way we can quit using GCC on Windows and just call the linker
directly?

This is a much harder thing to do. The linker flags are not being generated In just one place so supporting another linker interface would
either create a very intrusive mess or require a major refactoring.

That aside, the reason we use GCC is because we simply don't know how to link for the final executable. We don't know the implicit
dependencies GCC adds or where to find them. We don't understand the multi-lib or specs files concept either, so we can't currently
tell which crt to use. Basically any flag that affects ABI would be a potential issue, and we'd need to stop using linker scripts. We also
need to change/replace special symbols that ld would have normally provided, like the image base, etc.

In short yes it's doable, but I think it'll be much more work, fragile and the maintenance burden much higher.

Right, I briefly looked into doing this on Linux and concluded that 1) I wouldn't want to try go through the effort of getting it right; and 2) we don't want to have to maintain that effort in the future. In short, it opens us up to a whole new layer of things to get wrong.

austin resigned from this revision.Nov 9 2017, 11:31 AM