Page MenuHomeFreeBSD

Add a MULTILIB option to gcc{,48,49,5,5-devel,6-devel} for powerpc64
ClosedPublic

Authored by jmmv on Oct 20 2015, 12:45 PM.

Details

Summary

If the system is built with lib32 support (WITH_LIB32, which is the default),
building gcc from ports results in a compiler that can target both 64-bit and
32-bit binaries on powerpc64.

However, when lib32 support is disabled (WITHOUT_LIB32), gcc should only be
built with 64-bit support or otherwise the build fails.

To fix this, explicitly disable 32-bit support when /usr/lib32 is not present.
However, add a MULTILIB option (which is only defined for powerpc64 when
32-bit support is possible and defaults to yes to preserve the current
behavior) to allow the user to explicitly control this feature.


The above would be the commit message. Now, some commentary: I don't know
the gcc build system much, so this might be incorrect. According to the
documentation, the --enable/disable-multilib feature controls more than just
32-bit and 64-bit builds. However, because the only thing I see when running
"gcc -print-multi-lib" are targets for 32-bit and 64-bit (and, say, no soft
float), I think this is fine. Also, there does not seem to be a separate
flag to disable 32-bit support.

Note that if you want to play with enabling and later disabling lib32 on
a system, you will need D3923 or else the lib32 files won't be cleaned up.
Submitting D3923 is not a prerequisite for this commit.

Test Plan

Built gcc on amd64 with and without lib32, and with and without the new
MULTILIB option. I could not really observe differences. Did the same in
powerpc64, and "gcc -print-multi-lib" indeed showed different results. More
importantly, gcc now builds fine on powerpc64 out of the box.

Now testing gcc48 and gcc49. (The builds take a long time, so I'd rather
parallelize the review process with the actual build testing...)

I don't want to bother testing older gcc versions, hence why they are not
included in this patch.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 927
Build 927: arc lint + arc unit

Event Timeline

jmmv updated this revision to Diff 9532.Oct 20 2015, 12:45 PM
jmmv retitled this revision from to Add a MULTILIB option to gcc, gcc48 and gcc49..
jmmv updated this object.
jmmv edited the test plan for this revision. (Show Details)
jmmv added a reviewer: bapt.
jmmv added a comment.Oct 20 2015, 12:46 PM

I'm going to poke gerald@ by email to take a look and possibly to register for a Phabricator account.

+bdrewery. Let's add my primary mentor... my memory served me wrong and I got things backwards. (Now also waiting for a reply from Gerald via email; pointed him at here.)

bdrewery requested changes to this revision.Oct 20 2015, 3:44 PM
bdrewery edited edge metadata.
bdrewery added inline comments.
lang/gcc/Makefile
62–66

These can be spelled such as the following, without the PORT_OPTIONS check:

MULTILIB_CONFIGURE_ENV= enable_multilib=no
MULTILIB_CONFIGURE_OFF= --disable-multielib

The same change should be made to all 3 files.

This revision now requires changes to proceed.Oct 20 2015, 3:44 PM
jmmv updated this revision to Diff 9540.Oct 20 2015, 3:54 PM
jmmv edited edge metadata.

Use MULTILIB_CONFIGURE_ENABLE.

jmmv added inline comments.Oct 20 2015, 3:56 PM
lang/gcc/Makefile
62–66

That would not be correct because MULTILIB_CONFIGURE_ENV=enable_multilib=no would pass the reverse environment variable to the script when the feature is enabled.

I have changed this to use MULTILIB_CONFIGURE_ENABLE only. I have also dropped the use of MULTILIB_CONFIGURE_ENV: that was an artifact of a first attempt of my change, but on a second reading of the configure script, this should be unnecessary because --disable-multilib on the command line should have priority. Retrying builds to make sure.

bdrewery accepted this revision.Oct 20 2015, 4:00 PM
bdrewery edited edge metadata.

Be sure to get gerald's feedback too.

lang/gcc/Makefile
62–66

Yes you're right.

This revision is now accepted and ready to land.Oct 20 2015, 4:00 PM
jmmv updated this revision to Diff 9544.Oct 20 2015, 4:30 PM
jmmv edited edge metadata.

Make sure to disable multilib when MULTILIB is not defined.

This revision now requires review to proceed.Oct 20 2015, 4:30 PM
gerald added a subscriber: gerald.Oct 23 2015, 8:37 PM

Is it really appropriate to have this depend on the configuration of the build system, a la

​.if exists(/usr/lib32/libc.so)

? This does not appear to guarantee reproducible builds, does it?

(Adding Andreas Tobler as reviewer.)

jmmv added a comment.Oct 23 2015, 9:10 PM

I'd say depending on the presence of /usr/lib32 is fine. The builds will be reproducible for the host building the packages, and the standard binary packages should be built on a host without src.conf tweaks. Once you've modified MK_* settings in src.conf, you are in la la land.

andreast accepted this revision.Oct 24 2015, 8:20 PM
andreast edited edge metadata.

Ok with me.
I often used to build with --disable-multilib to save time. But I usually have a lib32/libc.so. So if this test works, fine.
powerpc64 is the only target in FreeBSD land which has multilib support in gcc. For all other targets the --disable-multilib is a nop.

This revision is now accepted and ready to land.Oct 24 2015, 8:20 PM

Okay, so it's okay to make this change for lang/gcc, lang/gcc49 and later.

Let's leave lang/gcc48 alone, there should be no reason to use this any more.

And why not lang/gcc5, lang/gcc5-devel, and lang/gcc6-devel?

As you apply this, please start with the newest port and work your way down
to older ones (just in case, best practice).

jmmv updated this revision to Diff 9697.Oct 25 2015, 2:13 PM
jmmv edited edge metadata.

Patch gcc 5 and 6 as well and only offer MULTILIB on powerpc64.

This revision now requires review to proceed.Oct 25 2015, 2:13 PM
jmmv added a comment.Oct 25 2015, 2:13 PM
In D3952#83418, @gerald wrote:

Okay, so it's okay to make this change for lang/gcc, lang/gcc49 and later.
Let's leave lang/gcc48 alone, there should be no reason to use this any more.

Then why is the port still in the tree? As long as it exists, it should be fixed to minimize surprises down the road. I wasted enough time testing builds between lang/gcc vs. lang/gcc48 when I first encountered these build issues because I did not realize they were pretty much the same port...

And why not lang/gcc5, lang/gcc5-devel, and lang/gcc6-devel?

Oh... just because I did not notice them! Patch updated.

jmmv added a comment.Oct 25 2015, 2:14 PM

Ok with me.
I often used to build with --disable-multilib to save time. But I usually have a lib32/libc.so. So if this test works, fine.
powerpc64 is the only target in FreeBSD land which has multilib support in gcc. For all other targets the --disable-multilib is a nop.

OK, that explains why I did not observe any differences in the packages being built on amd64. Because powerpc64 is the only platform in which this makes sense, the option should only exist in it; patch modified to do so.

jmmv retitled this revision from Add a MULTILIB option to gcc, gcc48 and gcc49. to Add a MULTILIB option to gcc{,48,49,5,5-devel,6-devel} for powerpc64.Oct 25 2015, 2:16 PM
jmmv updated this object.
jmmv removed a reviewer: bapt.

Re "why is lang/gcc48 still there?", there is a number of users interested in older versions,
which by definition do not make much sense for newer ports or newer uses, though.

One question, having though a bit more about the logic and our usual usage. Wouldn't
it make sense to always define this option, default it to false and only default it to true
when usr/lib32/libc.so exists?

So something like

 :
OPTIONS_DEFINE=...MULTILIB...
MULTILIB_DESC=                Build support for 32-bit and 64-bit targets where applicable
MULTILIB_CONFIGURE_ENABLE=        multilib
:

.if exists(/usr/lib32/libc.so)
​OPTIONS_DEFAULT_powerpc64=        MULTILIB
.endif

In this case we do _not_ need the general ​CONFIGURE_ARGS+=--disable-multilib any more.
And if someone actively sets the MULTILIB option, there will be an error when we are not able
to build (which makes sense I'd say).

What do you think?

(Note the adjusted _DESC, Can we simply say "targets" instead of "target types",
regardless of the outcome of this question?)

jmmv added a comment.Oct 26 2015, 2:25 AM
In D3952#83510, @gerald wrote:

Re "why is lang/gcc48 still there?", there is a number of users interested in older versions,
which by definition do not make much sense for newer ports or newer uses, though.

No, my question was: why is lang/gcc48 there when lang/gcc is pretty much a verbatim duplicate of it? If lang/gcc ought to work, so should lang/gcc48, especially because 4.8 is still the default gcc version.

One question, having though a bit more about the logic and our usual usage. Wouldn't
it make sense to always define this option, default it to false and only default it to true
when usr/lib32/libc.so exists?
So something like

 :
OPTIONS_DEFINE=...MULTILIB...
MULTILIB_DESC=                Build support for 32-bit and 64-bit targets where applicable
MULTILIB_CONFIGURE_ENABLE=        multilib
:
.if exists(/usr/lib32/libc.so)
​OPTIONS_DEFAULT_powerpc64=        MULTILIB
.endif

In this case we do _not_ need the general ​CONFIGURE_ARGS+=--disable-multilib any more.
And if someone actively sets the MULTILIB option, there will be an error when we are not able
to build (which makes sense I'd say).
What do you think?

This mostly works, but is suboptimal. Going this route allows the user to enable MULTILIB even when lib32 is not present, which would result in a build breakage. The MULTILIB option should only be exposed if the lib32 libraries are present.

(Note the adjusted _DESC, Can we simply say "targets" instead of "target types",
regardless of the outcome of this question?)

Done.

jmmv updated this revision to Diff 9706.Oct 26 2015, 2:26 AM

Fix description according to suggestion.

jmmv added a comment.Oct 28 2015, 1:12 AM

ping reviewers

Yes, you have already approved the revision before, but you should do so again because it has potentially changed quite a bit since you did that.

Thank you

I still am not fully convinced that the presence of OPTIONs should depend
on the host system, but since none of the portmgrs are concerned:

Approved-by: gerald (maintainer)

(Not sure how to add myself as Approver in this tool, but as maintainer of
these ports, that's all you should need. ;-)

Just swap MULTILIB_CONFIGURE_ENABLE and MULTILIB_DESC as you
commit, and start with gcc6-devel and gcc5-devel, giving those several days
before you go for the rest.

andreast accepted this revision.Oct 30 2015, 9:35 PM
andreast edited edge metadata.

I tested on my side and it is fine so far. I'd like to have this in since I have a follow up for enable/disable java for powerpc64. And also, gcc-6.0 can not be bootstrapped with the default gcc (gcc-4.2.1). it needs at newer one which must be < 5.2. gcc-5.2 is broken when it comes to linking. I guess PIE support.

So, I'm fine for all versions. Thanks
Andreas

This revision is now accepted and ready to land.Oct 30 2015, 9:35 PM
This revision was automatically updated to reflect the committed changes.
jmmv added a comment.Nov 2 2015, 2:58 AM

Thank you everyone. I've done the last stylistic change requested by gerald and committed the changes to gcc5-devel and gcc6-devel. The rest will follow later in the week.

gerald: To add yourself as reviewer, you should be able to click on the "Edit revision" button at the top right of the page and modify the list of reviewers.

jmmv reopened this revision.Nov 2 2015, 3:09 AM

I only committed a subset of the changes, so let's reopen this. The automatic closing caused by a commit wasn't desirable in this case.

This revision is now accepted and ready to land.Nov 2 2015, 3:09 AM
This revision was automatically updated to reflect the committed changes.

Why did you allow to enable multilib for all archs on gcc4.9/gcc4.8? It is still a no-op for all archs beside powerpc64.

gerald added a comment.Nov 8 2015, 8:32 PM

No, my question was: why is lang/gcc48 there when lang/gcc is pretty much a verbatim duplicate of it? If lang/gcc ought to work, so should lang/gcc48, especially because 4.8 is still the default gcc version.

lang/gcc is the general, default port that most people will want to use (and should).

Right now it is based on GCC 4.8, but there is a push to re-base on GCC 4.9, and
later GCC 5.

jmmv reopened this revision.Nov 8 2015, 8:33 PM

I messed up the commits and this was only intended to be enabled on powerpc64. I think I committed an older version of the diff. I have undone the committed changes and will take a closer look at what happened.

This revision is now accepted and ready to land.Nov 8 2015, 8:33 PM
This revision was automatically updated to reflect the committed changes.