Make ghc-split more modern
AbandonedPublic

Authored by DemiMarie on Nov 22 2016, 6:56 PM.

Details

Summary

ghc-split was written using old Perl 4 conventions. This patch
modifies it to conform with modern Perl standards.

Test Plan

GHC CI

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Branch
cleanup-dllsplit
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 12283
Build 15878: [GHC] Linux/amd64: Patch building
Build 15877: arc lint + arc unit
DemiMarie updated this revision to Diff 9581.Nov 22 2016, 6:56 PM
DemiMarie retitled this revision from to Make ghc-split more modern.
DemiMarie updated this object.
DemiMarie edited the test plan for this revision. (Show Details)
bgamari edited edge metadata.Nov 24 2016, 1:48 PM

I'm afraid my Perl isn't so good. Have you tested this, @DemiMarie?

Phyx added a subscriber: Phyx.EditedNov 24 2016, 2:30 PM

Thanks for trying to clean this up!

One concern though, we bundle a very ancient version of perl with the Windows bindist. Does this still work using it?

This is perl, v5.6.1 built for MSWin32-x86

The reason we do so is because it's a very lightweight dependency and doesn't grow the binary immensely like modern perls.

Also the patch seems to drop support for mips and hppa?

I don't think it's enough to rely on harbormaster to test this. Since I'm not sure if the test coverage for this is very large. Also harbormaster does a small amount of testing for the platforms that use this script.

With my last attempt I requested a maintainer for each platform to send me some test data gathered with

ghc -split-objs -keep-tmp-files -v <hs_file>

The files I asked for were compiler/utils/GraphOps.hs and compiler/deSugar/Coverage.hs.

The reason is they're big and complex enough to generate enough splits to be a decent test and they don't use CPP so they can be compiled standalone.

It's hard to generate a small test case which will get the right assembly for each platform so that interesting splits are triggered.

If it helps, the expected SPARC and MIPS output I received are https://1drv.ms/f/s!AuQz_u-9HaJPmZcdBWwOaIfXZ29oAw here.

The input files .split and .split_s are in there as well. You should check to see if your version still produces the correct output.

DemiMarie added a comment.EditedNov 24 2016, 6:31 PM
In D2748#80024, @Phyx wrote:

Thanks for trying to clean this up!

One concern though, we bundle a very ancient version of perl with the Windows bindist. Does this still work using it?

This is perl, v5.6.1 built for MSWin32-x86

The reason we do so is because it's a very lightweight dependency and doesn't grow the binary immensely like modern perls.

This code uses ${^PREMATCH} and ${^POSTMATCH}, which don't exist on Perl < 5.10. I will fix this. will need to be fixed.

Also the patch seems to drop support for mips and hppa?

The information collected by collectExports_mips and collectExports_hppa was never used. Therefore, I deleted it as unused code.

DemiMarie updated this revision to Diff 9609.Nov 24 2016, 6:55 PM
DemiMarie edited edge metadata.

Support Perl 5.6

DemiMarie updated this revision to Diff 9615.Nov 25 2016, 12:41 PM
DemiMarie edited edge metadata.

Only include this patch, not D2732.

DemiMarie updated this revision to Diff 9616.Nov 25 2016, 1:09 PM
DemiMarie edited edge metadata.

Fix a silly regexp bug and do more cleanups.

DemiMarie updated this revision to Diff 9617.Nov 25 2016, 1:10 PM
DemiMarie edited edge metadata.

Try to make Phabricator see the splitter as Perl

DemiMarie updated this revision to Diff 9618.Nov 25 2016, 1:21 PM
DemiMarie edited edge metadata.

Revert previous change

I'm afraid my Perl isn't so good. Have you tested this, @DemiMarie?

I haven't, but GHC CI has. I am worried about breaking compatibility with some unusual systems though.

Incidentally, is the splitter required by any platform? I thought that we could use --gc-sections on all platforms now, making the splitter redundant. If so, we can just drop it.

DemiMarie updated this revision to Diff 9726.Dec 1 2016, 11:32 AM
DemiMarie edited edge metadata.

Delete more dead code

Splitting is not supported when compiling via LLVM, nor is it
supported in unregisterised mode. Therefore, it is only
supported when using the native code generator

DemiMarie updated this revision to Diff 9727.Dec 1 2016, 11:36 AM
DemiMarie edited edge metadata.

Restore support for splitting on Sparc

DemiMarie updated this revision to Diff 9728.Dec 1 2016, 11:39 AM
DemiMarie edited edge metadata.

Change printf to print

Fix a bug in case there was a % in some variables

simonmar edited edge metadata.Dec 1 2016, 12:21 PM

Aren't we deleting the splitter? Is it worth cleaning it up first?

Aren't we deleting the splitter? Is it worth cleaning it up first?

Probably not. Since virtually every OS that runs on SPARC or PowerPC uses ELF (and thus can use the GNU linker and hence -split-sections), and -split-sections works on 64-bit Windows, the only remaining use of the splitter is for 32-bit Windows. Even that will go away once binutils bugs are fixed.

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

Aren't we deleting the splitter? Is it worth cleaning it up first?

Probably not. Since virtually every OS that runs on SPARC or PowerPC uses ELF (and thus can use the GNU linker and hence -split-sections), and -split-sections works on 64-bit Windows, the only remaining use of the splitter is for 32-bit Windows. Even that will go away once binutils bugs are fixed.

@DemiMarie, Perhaps you could abandon this if you think it has outlived its usefulness?

This revision now requires changes to proceed.Dec 6 2016, 12:22 AM
DemiMarie abandoned this revision.Dec 6 2016, 11:54 AM

Superseded by D2768