This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] [CMake] Removing expand-vars.pl in favor of CMake's configure_file()
ClosedPublic

Authored by jlpeyton on Aug 20 2015, 1:35 PM.

Details

Reviewers
chandlerc
hfinkel
Summary

Currently, the libomp CMake build system uses a Perl script to configure files (tools/expand-vars.pl). This patch replaces the use of the Perl script by using CMake's configure_file() function. The major changes include:

  1. *.var has every $KMP_* variable changed to @LIBOMP_*@
  2. kmp_config.h.cmake is a new file which contains all the feature macros and #cmakedefine lines
  3. Most of the -D lines have been moved from LibompDefinitions.cmake but some OS specific MACROs (e.g., _GNU_SOURCE) remain.
  4. All expand-vars.pl related logic is removed from the CMake files.

One important note about this change is that it breaks the old Perl+Makefile build system because it can't create kmp_config.h properly.

Diff Detail

Repository
rL LLVM

Event Timeline

jlpeyton updated this revision to Diff 32728.Aug 20 2015, 1:35 PM
jlpeyton retitled this revision from to [OpenMP] [CMake] Removing expand-vars.pl in favor of CMake's configure_file().
jlpeyton updated this object.
jlpeyton added reviewers: chandlerc, hfinkel.
jlpeyton set the repository for this revision to rL LLVM.
jlpeyton added a subscriber: openmp-commits.

Working good in general, some remarks inline.

(note: There is still the file libiomp.rc.var which I think can be deleted anyway)

runtime/src/libomp.rc.var
15–16

Couldn't this be replaced with #include "kmp_config.h" ...

28

... and this be #if KMP_DEBUG?

43–53

The README still says LLVM* OpenMP*. Was this change intended?

jlpeyton updated this revision to Diff 32958.Aug 24 2015, 8:00 AM

Updating diff to address Jonas's concerns.

Couldn't this be replaced with #include "kmp_config.h" ...

Replaced.

... and this be #if KMP_DEBUG?

Replaced.

The README still says LLVM* OpenMP*. Was this change intended?

Not intended, changed it to LLVM* OpenMP* again.

Thanks Jonas for pointing these out.

Also, I removed a commented line of code (line 155) from runtime/src/CMakeLists.txt
#set_source_files_properties(libomp.rc PROPERTIES EV_COMPILE_DEFINITIONS "-D KMP_FILE=${LIBOMP_LIB_FILE}" GENERATED TRUE)

hfinkel accepted this revision.Aug 27 2015, 7:32 PM
hfinkel edited edge metadata.

LGTM.

As this breaks the old gmake+perl system, please remove it and any other files that makes unnecessary.

This revision is now accepted and ready to land.Aug 27 2015, 7:32 PM
jlpeyton closed this revision.Aug 28 2015, 2:21 PM

Committed r246314 and r246317 (forgot to add kmp_config.h.cmake)