New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make -linkall applicable to single compilation units #1009

Closed
wants to merge 3 commits into
base: trunk
from

Conversation

Projects
None yet
5 participants
@xavierleroy
Contributor

xavierleroy commented Jan 13, 2017

The -linkall option of ocamlc/ocamlopt was introduced a long time ago to deactivate the link-time optimization of libraries, whereas objects (.cmo/.cmx) contained in libraries (.cma/.cmxa) are not linked if none of their definitions is referenced.

At link-time, -linkall causes all objects of all libraries to be linked and included in the final executable.

At library construction time (-a option), -linkall causes the "always link" flag to be put on all objects in the library, so that when the library is linked, all its objects are linked.

The present pull request extends this behavior to single objects / compilation units. Compiling with -c -linkall causes the production of an object file with the "always link" flag set. When put in a library, this object retains its "always link" flag, even if -linkall is not given at library construction time and the other objects in the library do not have the "always link" flag.

This new behavior can be useful for libraries that contain one compilation unit that performs important initializations (e.g. for a mixed OCaml/C library, initializing global data structures on the C side) and other units that don't:

  • Not using -linkall anywhere in the library build can cause data structures to remain uninitialized, see https://caml.inria.fr/mantis/view.php?id=7459 for an example of this bug.
  • Building the library with -a -linkall causes all units to be linked all the time, which can be costly in code size.
  • Compiling the first unit with -c -linkall and building the library without -linkall ensures that the initialization is always performed, but the rest of the library is linked only when actually used.

This pull request just introduces the mechanism. If it is judged favorably, we'll see about using it to make some of the otherlibs/ more robust w.r.t. initialization.

Make -linkall applicable to single compilation units
With this commit, "ocamlc -c -linkall" or "ocamlopt -c -linkall" produce object files that have the "force link" flag set.  Once put in library files (.cma/.cmxa), these object files will always be linked when the library is linked, even if no definition from the object file is referenced.  However, other object files in the library can still be removed if none of their definitions are referenced.

In this respect, we get finer-grained control on which object files should always be linked, typically because they contain important initialization code.
@gasche

This comment has been minimized.

Show comment
Hide comment
@gasche

gasche Jan 14, 2017

Member

I agree that this is a desirable feature, and the implementation is very simple and non-invasive.

(The CI failed for random network-access reasons, I just restarted it.)

Member

gasche commented Jan 14, 2017

I agree that this is a desirable feature, and the implementation is very simple and non-invasive.

(The CI failed for random network-access reasons, I just restarted it.)

@kayceesrk

This comment has been minimized.

Show comment
Hide comment
@kayceesrk

kayceesrk Jan 14, 2017

Contributor

The PR looks good to me. This feature will help avoid the non-intuitive behaviour with the systhreads library.

Contributor

kayceesrk commented Jan 14, 2017

The PR looks good to me. This feature will help avoid the non-intuitive behaviour with the systhreads library.

@gasche

This comment has been minimized.

Show comment
Hide comment
@gasche

gasche Jan 14, 2017

Member

@xavierleroy, could you add a Changes entry? (This makes sense as a user-facing change). I think the category "Compiler user-interface and warnings" would be a good choice.

Another thing that would be useful is to port the manual change you made to the man pages man/ocaml{c,opt}.m.

Member

gasche commented Jan 14, 2017

@xavierleroy, could you add a Changes entry? (This makes sense as a user-facing change). I think the category "Compiler user-interface and warnings" would be a good choice.

Another thing that would be useful is to port the manual change you made to the man pages man/ocaml{c,opt}.m.

bschommer and others added some commits Jan 13, 2017

PR#7460, GPR#1011: Fix uncaught Arg.Bad exception in compenv.ml
This was a regression in 4.04.0, where passing files with unknown
extension (foo.xwds) to the compiler raises an uncaught Arg.Bad
exception.
@xavierleroy

This comment has been minimized.

Show comment
Hide comment
@xavierleroy

xavierleroy Jan 15, 2017

Contributor

@gasche, your wish is my command.

Contributor

xavierleroy commented Jan 15, 2017

@gasche, your wish is my command.

@gasche

This comment has been minimized.

Show comment
Hide comment
@gasche

gasche Jan 15, 2017

Member

You ended up with a commit from trunk in the branch, I'm not sure how. (I think to rebase a branch the most natural process is to first update your trunk with git checkout trunk; git pull and then rebase the branch on top of trunk git checkout linkall; git rebase trunk). I merged the PR manually.

Member

gasche commented Jan 15, 2017

You ended up with a commit from trunk in the branch, I'm not sure how. (I think to rebase a branch the most natural process is to first update your trunk with git checkout trunk; git pull and then rebase the branch on top of trunk git checkout linkall; git rebase trunk). I merged the PR manually.

@gasche gasche closed this Jan 15, 2017

@xavierleroy xavierleroy deleted the linkall branch Jan 17, 2017

@damiendoligez

This comment has been minimized.

Show comment
Hide comment
@damiendoligez

damiendoligez Mar 1, 2017

Member

Note that this was MPR#6409.

Member

damiendoligez commented Mar 1, 2017

Note that this was MPR#6409.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment