Add missing -function-sections in Makefile in otherlibs by gretay-js · Pull Request #9165 · ocaml/ocaml

gretay-js · 2019-12-06T13:47:41Z

The flag -function-sections was not passed to ocamlopt as needed in two of the Makefiles under otherlibs. These two Makefiles don't include the common Makefile which takes care of adding this option.

mshinwell · 2019-12-06T13:58:41Z

I suggest this is backported to 4.10.

dra27 · 2019-12-07T09:31:57Z

The ultimate goal is to have only one common Makefile for otherlibs, but that’s a ton of work. However, rather than papering this over again, could this be done either by shifting these common definitions all the way up to Makefile.common.in (if that can work there - is this flag wanted everywhere ocamlopt is invoked?) or by splitting Makefile.otherlibs.common into parts which really are common to all three templates (macro definitions) and only for the “simple” libraries (rules for unix, str, etc.)?

gretay-js · 2020-04-06T15:30:09Z

@dra27 I'm sorry for leaving your comment unanswered for so long (not sure how I missed it).

As you suggested, I moved the definition of OPTCOMPFLAGS to Makefile.common.in. It is the only place where FUNCTION_SECTIONS variable is checked and the flag -function-sections is added to OPTCOMPFLAGS. So far, these changes preserve the behavior we had before.

In addition, the last commit also moves the check for FLAMBDA to Makefile.common.in in order to add "-O3" to OPTCOMPFLAGS. It preserves the behavior of OPTCOMPFLAGS in stdlib and otherlib. It changes OPTCOMPFLAGS in the "main" Makefile to use "-O3" option when configured with FLAMBDA. The tests pass, it doesn't seem to take longer, and it is nice and uniform. Is there a particular reason it shouldn't be compiled with -O3?

dra27

Thanks for the changes, and sorry for my lag approving!

dra27

Given that it still short a Changes entry, 3 nits of extra blank lines which I don't think are needed!

Makefile

Makefile.common.in

stdlib/Makefile

dra27 · 2020-04-16T16:44:14Z

Good to go once it has a Changes entry and I agree with @mshinwell that this should go to 4.10 as well.

dra27 · 2020-04-17T12:23:40Z

@Octachron - if this is going in 4.10, does that the mean the Changes entry on trunk should also be the "4.10 Maintenance Branch" bit, or in the "Working Version"(/4.11) bit?

dra27 · 2020-04-17T12:24:16Z

Thanks for the updates @gretay-js - I'll merge and cherry-pick this once CI is green!

gretay-js · 2020-04-17T12:24:37Z

Thank you!

dra27 · 2020-04-20T13:06:23Z

Ping @Octachron, just to confirm which Changes section this can go in - I'll fix that after merge

Octachron · 2020-04-20T13:17:43Z

Sorry, I missed your earlier question. Since we are planning to have a 4.10.1 release, before the 4.11.0 release, I think it is fine to have the entry in the future 4.10.1 section.

dra27 · 2020-04-20T14:31:04Z

Thanks!

Add missing -function-sections in Makefile in otherlibs (cherry picked from commit 06cc08e)

ocaml#8938 from upstream trunk (cherry-pick 4353c75) ocaml#9090 from upstream trunk (cherry-pick fe7c8ed) ocaml#9097 from upstream trunk (cherry-pick 6daaf62) ocaml#8939 from gretay-js/save-linear (cherry-pick 1200de9) ocaml#9003 from gretay-js/start-from-emit (cherry-pick 21d5d26)

ocaml#8938 from upstream trunk (cherry-pick 4353c75) ocaml#9090 from upstream trunk (cherry-pick fe7c8ed) ocaml#9097 from upstream trunk (cherry-pick 6daaf62) ocaml#8939 from gretay-js/save-linear (cherry-pick 1200de9) ocaml#9003 from gretay-js/start-from-emit (cherry-pick 21d5d26) Add linear_format to dune file

ocaml#8938 from upstream trunk (cherry-pick 4353c75) ocaml#9090 from upstream trunk (cherry-pick fe7c8ed) ocaml#9097 from upstream trunk (cherry-pick 6daaf62) ocaml#8939 from gretay-js/save-linear (cherry-pick 1200de9) ocaml#9003 from gretay-js/start-from-emit (cherry-pick 21d5d26) Add linear_format to dune file Update .depend

stedolan added a commit to janestreet/ocaml that referenced this pull request Mar 17, 2020

ocaml#9165 from gretay-js/fs-otherlibs (cherry-pick 3f33022)

5e30bbc

dra27 added this to the 4.11 milestone Apr 16, 2020

dra27 added the bug label Apr 16, 2020

dra27 approved these changes Apr 16, 2020

View reviewed changes

dra27 reviewed Apr 16, 2020

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Makefile.common.in Outdated Show resolved Hide resolved

stdlib/Makefile Outdated Show resolved Hide resolved

gretay-js force-pushed the fs-otherlibs branch from 7c503a6 to 839d98b Compare April 17, 2020 12:16

gretay-js added 6 commits April 20, 2020 13:56

Add -function-sections to missing folder in otherlibs

ec03b06

Define OPTCOMPFLAGS in Makefile.common.in

7465e9f

Check FUNCTION_SECTIONS in Makefile.common.in to update OPTCOMPFLAGS

44af4ae

Check FLAMBDA in Makefile.common.in to update OPTCOMPFLAGS

7271919

Remove stray newlines

b823c8d

Add an entry to Changes

0ba942c

gretay-js force-pushed the fs-otherlibs branch from 839d98b to 0ba942c Compare April 20, 2020 12:59

Move Changes entry

5150303

dra27 merged commit 06cc08e into ocaml:trunk Apr 20, 2020

dra27 added a commit that referenced this pull request Apr 20, 2020

Merge pull request #9165 from gretay-js/fs-otherlibs

ce4cdaa

Add missing -function-sections in Makefile in otherlibs (cherry picked from commit 06cc08e)

gretay-js pushed a commit to gretay-js/ocaml that referenced this pull request Jun 4, 2020

ocaml#9165 from gretay-js/fs-otherlibs (cherry-pick 3f33022)

f54c246

Octachron mentioned this pull request Aug 11, 2020

4.11 beta3+flambda: stack overflow when compiling uunf #9839

Closed

dra27 added a commit to dra27/ocaml that referenced this pull request Aug 12, 2020

Revert the -O3 portion of ocaml#9165

d4a9543

dra27 added a commit to dra27/ocaml that referenced this pull request Aug 12, 2020

Revert the -O3 portion of ocaml#9165

cf970dc

dra27 added a commit to dra27/ocaml that referenced this pull request Aug 12, 2020

Revert the -O3 portion of ocaml#9165

7d89ff1

dra27 added a commit to dra27/ocaml that referenced this pull request Aug 12, 2020

Revert the -O3 portion of ocaml#9165

c397d67

Conversation

gretay-js commented Dec 6, 2019

Uh oh!

mshinwell commented Dec 6, 2019

Uh oh!

dra27 commented Dec 7, 2019

Uh oh!

gretay-js commented Apr 6, 2020

Uh oh!

dra27 left a comment

Choose a reason for hiding this comment

Uh oh!

dra27 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dra27 commented Apr 16, 2020

Uh oh!

dra27 commented Apr 17, 2020

Uh oh!

dra27 commented Apr 17, 2020

Uh oh!

gretay-js commented Apr 17, 2020

Uh oh!

dra27 commented Apr 20, 2020

Uh oh!

Octachron commented Apr 20, 2020

Uh oh!

dra27 commented Apr 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dra27 left a comment •

edited

Loading