Page MenuHomePhabricator

Add CMAKE variable to enable creation of static OpenMP libraries instead of dynamic ones
ClosedPublic

Authored by jmellorcrummey on Jan 24 2016, 12:15 PM.

Details

Summary

When building executables for Cray supercomputers, statically-linked executables are preferred. This patch makes it possible to build the OpenMP runtime as an archive for building statically-linked executables.

The patch adds the flag LIBOMP_SHARED_LIBRARY, which defaults to true. When true, a build of the OpenMP runtime yields dynamic libraries. When false, a build of the OpenMP runtime yields static libraries. There is no setting that allows both kinds of libraries to be built.

Diff Detail

Event Timeline

jmellorcrummey retitled this revision from to Add CMAKE variable to enable creation of static OpenMP libraries instead of dynamic ones.
jmellorcrummey updated this object.
jmellorcrummey added a reviewer: jlpeyton.
jmellorcrummey set the repository for this revision to rL LLVM.
jlpeyton edited edge metadata.Jan 26 2016, 8:37 AM

Add the new variable to Build_With_CMake.txt with note that it isn't available on Windows.

runtime/CMakeLists.txt
280–283

This is a minor nit, but can this be renamed to LIBOMP_STATIC_LIBRARY which is FALSE by default. Setting a feature to true to enable the feature seems a little more intuitive for the user. Also, make a note in the comment that this feature isn't available on Windows, and put below this:

if(WIN32 AND LIBOMP_STATIC_LIBRARY)
  libomp_error_say("Static libraries requested but not available on Windows")
endif()
hfinkel added a subscriber: hfinkel.Feb 3 2016, 8:18 PM
hfinkel added inline comments.
runtime/CMakeLists.txt
280–283

This is a minor nit, but can this be renamed to LIBOMP_STATIC_LIBRARY which is FALSE by default.

FWIW, the libc++ option is named:

option(LIBCXX_ENABLE_SHARED "Build libc++ as a shared library." ON)

and the LLVM option is named (strangely without an LLVM_ prefix):

option(BUILD_SHARED_LIBS
  "Build all libraries as shared libraries instead of static" OFF)

so it might be more consistent to leave the option with the proposed sense. We might even want to mirror libc++ more closely and name it LIBOMP_ENABLE_SHARED.

I don't, however, feel strongly about it either way. I would like to get this in, however; it is something we need.

jmellorcrummey edited edge metadata.

Address Jonathan's comments:

  • Renamed flag LIBOMP_SHARED_LIBRARY to LIBOMP_STATIC_LIBRARY.
  • Add documentation for the flag to runtime/Build_With_CMake.txt
jlpeyton added inline comments.Feb 4 2016, 8:28 AM
runtime/CMakeLists.txt
280–283

Ok, I do think we should mimic libc++ to keep the interface similar. Sorry John, but can you change it to LIBOMP_ENABLE_SHARED which is on be default?

Just an FYI,
The BUILD_SHARED_LIBS variable is actually a standard CMake variable (with no CMAKE_ prefix).

jmellorcrummey marked an inline comment as done.

Update revision to meet Jonathan's requirement of the day: change control flag to LIBOMP_ENABLE_SHARED.

jlpeyton accepted this revision.Feb 4 2016, 11:15 AM
jlpeyton edited edge metadata.

Thanks. LGTM.

This revision is now accepted and ready to land.Feb 4 2016, 11:15 AM
This revision was automatically updated to reflect the committed changes.