Page MenuHomeFreeBSD

lang/ghc: port to powerpc64
ClosedPublic

Authored by arrowd on Jan 18 2019, 11:38 AM.

Details

Summary

bootstrap for 12.0 available here http://mikael.urankar.free.fr/FreeBSD/powerpc64/ghc-8.6.3-boot-powerpc64-freebsd.tar.xz

Patches come from https://gitlab.haskell.org/trommler/ghc/commits/wip/T15916
Most of them are already commited upstream.

these 2 patches are not yet committed:
https://ghc.haskell.org/trac/ghc/ticket/15411 (Disable-unboxed-arrays.patch)
https://gitlab.haskell.org/trommler/ghc/commit/19731a77ed203870f76a53eaf01758efbb5144d3
For the record, upstream PR for FreeBSD/ppc64: https://ghc.haskell.org/trac/ghc/ticket/15916

Test Plan

ok on 12.0 ppc64, 8.6.2 and 8.6.3 (the 8.4.x branch is too old)
ok on amd64

the 'llvm backend' is broken (not sure if it's supposed to work, I'll submit a pr upstream)

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mikael created this revision.Jan 18 2019, 11:38 AM
mikael updated this revision to Diff 53037.Jan 20 2019, 1:05 PM
mikael retitled this revision from lang/ghc: WIP port to powerpc64 to lang/ghc: port to powerpc64.
mikael edited the summary of this revision. (Show Details)
mikael edited the test plan for this revision. (Show Details)
arrowd added inline comments.Jan 22 2019, 7:36 AM
lang/ghc/bsd.ghc.mk
13 ↗(On Diff #53037)

Why do we need that?

mat added inline comments.Jan 23 2019, 8:44 AM
lang/ghc/Makefile
21 ↗(On Diff #53037)

Why +=?

lang/ghc862/Makefile
21 ↗(On Diff #53037)

Why +=?

mikael added inline comments.Jan 23 2019, 12:57 PM
lang/ghc/Makefile
21 ↗(On Diff #53037)

I don't know, it seems weird to have ONLY_FOR_ARCHS= powerpc64, it looks like the port is restricted to ppc64 until you look at bsd.ghc.mk

lang/ghc/bsd.ghc.mk
13 ↗(On Diff #53037)

I need a recent compiler, I probably just need compiler:c11 (or put use_gcc in the ppc64 'if block')

mikael updated this revision to Diff 53111.Jan 23 2019, 4:53 PM

address comments

tcberner added inline comments.Jan 27 2019, 11:11 AM
lang/ghc/Makefile
18 ↗(On Diff #53111)

^ what?

tcberner added inline comments.Jan 27 2019, 11:12 AM
lang/ghc/bsd.ghc.mk
8 ↗(On Diff #53111)

^ why not add powerpc64 here? instead having that confusing line in lang/ghc/Makefile

mikael added inline comments.Jan 27 2019, 11:39 AM
lang/ghc/Makefile
18 ↗(On Diff #53111)

mat didn't like the ONLY_FOR_ARCHS+= ppc64 notation.
you don't like ONLY_FOR_ARCHS= ppc64
should I remove O_F_A in bsd.ghc.mk and list the allowed arches in each Makefile?

lang/ghc/bsd.ghc.mk
8 ↗(On Diff #53111)

It's only for ghc-8.6.x

mat added inline comments.Jan 28 2019, 1:57 PM
lang/ghc/Makefile
18 ↗(On Diff #53111)

I did not say I did not liked it, I asked why, because the variable was not set before, and doing += means you are adding to it, which was weird, because it was not defined before.

You should add a comment saying other archs are added later, because it looks weird like this.

mikael updated this revision to Diff 53327.Jan 28 2019, 6:00 PM

address comment

tcberner accepted this revision.Jan 29 2019, 7:26 PM

Looks good to me.

This revision is now accepted and ready to land.Jan 29 2019, 7:26 PM
arrowd commandeered this revision.Feb 26 2019, 11:16 AM
arrowd edited reviewers, added: mikael; removed: arrowd.
arrowd updated this revision to Diff 54412.Feb 26 2019, 11:18 AM

Rework ONLY_FOR_ARCHS handling.

This revision now requires review to proceed.Feb 26 2019, 11:18 AM
mat added inline comments.Feb 26 2019, 11:31 AM
lang/ghc/bsd.ghc.mk
183 ↗(On Diff #54412)

I wonder if this should not be changed to positive tests with the architectures that actually need this instead of excluding the ones that do not.

239–240 ↗(On Diff #54412)

Same here.

arrowd updated this revision to Diff 54413.Feb 26 2019, 11:37 AM
arrowd marked 2 inline comments as done.

Simplify checks.

I want to note that this Diff was about adding powerpc64 support to lang/ghc. It is seems to be moving into "refactor bsd.ghc.mk" direction now. I admit there is much to be done here, but it can be done iteratively and later.

mikael accepted this revision.Feb 26 2019, 12:12 PM
This revision is now accepted and ready to land.Feb 26 2019, 12:12 PM
arrowd updated this revision to Diff 54569.Mar 1 2019, 5:50 AM

Fix condition.

This revision now requires review to proceed.Mar 1 2019, 5:50 AM
tcberner added inline comments.Mar 1 2019, 6:08 AM
lang/ghc/bsd.ghc.mk
91 ↗(On Diff #54569)

For a future revision, couldnt we do something like

_BOOT_GHC_VESRSION_amd64= 8.6.4
_BOOT_GHC_VESRSION_i386=8.6.4
[...]
_BOOT_GHC_VERSION_${ARCH}?=8.4.3
BOOT_GHC_VERSION=${_BOOT_GHC_VERSION_${ARCH}}
arrowd updated this revision to Diff 54571.Mar 1 2019, 7:32 AM

Final fix.

You seem to have forgotten adjusting ONLY_FOR_ARCHS :-)

I've fixed that locally and will see what happens overnight.

arrowd added a comment.Mar 1 2019, 9:04 AM

You seem to have forgotten adjusting ONLY_FOR_ARCHS :-)

Hum? Where?

tcberner added inline comments.Mar 1 2019, 8:43 PM
lang/ghc/bsd.ghc.mk
8 ↗(On Diff #54571)

^ this seems to breaks it:

you cannot include bsd.port[.pre].mk twice
arrowd updated this revision to Diff 54651.Mar 3 2019, 7:17 PM

Bring IGNORE= line back to its place, because checking for OSVERSION at the
top requires including bsd.port.pre.mk, but it should be included after all
other variables are set.

Hum? Where?

OK, I understand it now. The assingments were confusing.

tcberner accepted this revision.Mar 4 2019, 5:06 AM

looks good to me again

This revision is now accepted and ready to land.Mar 4 2019, 5:06 AM
This revision was automatically updated to reflect the committed changes.