CMake overhaul

Hello everyone,

I have been working on overhauling the CMake build system for the LLVM OpenMP Runtime library. Instead of sending 1000 small updates, I decided to do a complete refactoring/reorganizing/renaming/you-name-it-I-did-it. All the interface variables are still the same (LIBOMP_OS, LIBOMP_ARCH, etc.). So there was no change there. Here are the major changes:

  1. Renaming internal variables/macros/functions – everything has libomp_ or LIBOMP_ prefixed to it unless maybe if it is a local variable/parameter inside a function/macro.

  2. config-ix.cmake added – compiler flag checks, linker flag checks, some feature checks, threading-model check. This means cmake/${CMAKE_C_COMPILER_ID}/*Flags.cmake directories/files are deleted.

  3. All files inside cmake/ subdirectory are renamed to Libompxyz.cmake

  4. I cut off most of the vestigial parts that were a part of the translation of build.pl system. There are still a few that remain like LibompExports.cmake (copying things to exports/ subdirectory) and LibompMicroTests.cmake (small tests to run on the just created library. This is invoked with make libomp-micro-tests), but these can easily be cut off because they are in their own files and surrounded by an if() guard in the CMakeLists.txt file.

  5. Added LLVM-architecture detection if we are part of an LLVM build.

  6. If you want, you can use LIBOMP_ARCH=i386 or LIBOMP_ARCH=x86_64 instead of LIBOMP_ARCH=32 or LIBOMP_ARCH=32e (although 32 and 32e still exist)

  7. Unused functions/macros have been cut away.

This is easier to review if you just patch and take a look at files runtime/CMakeLists.txt, runtime/src/CMakeLists.txt, cmake/Libomp*.cmake:

$ patch –p1 –E < CMake_overhaul_v1.patch

[-E removes empty files created by the patch]

I’ve tested this out on x86 Linux, Mac, MIC, Windows and it works, but I can’t test it on other architectures.

Leave feedback, recommendations.

– Johnny

CMake_overhaul_v1.patch (192 KB)

Jonathan,
       The build on x86_64-apple-darwin14, with openmp placed in-tree
at llvm/projects, completes fine here. The only difference I see with
these changes is that the libgomp.dylib symlink pointing at
libomp.dylib is no longer created during 'make install'. Is that
change intended?
              Jack

Hello everyone,

I have been working on overhauling the CMake build system for the LLVM OpenMP Runtime library. Instead of sending 1000 small updates, I decided to do a complete refactoring/reorganizing/renaming/you-name-it-I-did-it. All the interface variables are still the same (LIBOMP_OS, LIBOMP_ARCH, etc.). So there was no change there.

FWIW, I’m excited to see this work.

One question, would it be OK to use Phabricator for the review? While I agree and plan on patching and looking at the patch directly, creating comments on specific parts of the change is much easier for me with Phabricator, so I’ll be able to review it there faster.

Have you looked much at the libc++ and/or compiler-rt CMake build? I’ve suggested this a few times before, but it may provide a useful reference for some simplifying concepts.

Relatedly, I think it would be good to discuss this with Chris Bieneman who has been working on the compiler-rt CMake build. Specifically, I don’t know what the host compiler requirements are for the runtime, but some of our runtimes require that they are built with the just-built-Clang compiler.

My final high level question (which probably just reflects my not knowing the current status) is what the progress is on using the lit test infrastructure? I don’t want to add a bunch of comments about that here if it really needs to be dealt with separately, but I suspect one of the key issues is going to be integrating the cmake build with the lit test infrastructure and how that works.

Here are the major changes:

  1. Renaming internal variables/macros/functions – everything has libomp_ or LIBOMP_ prefixed to it unless maybe if it is a local variable/parameter inside a function/macro.

So, looking at the source code layout, what is the plan around the offload tree? I find the structure here confusing.

We have a ‘runtime’ directory, and an ‘offload’ directory. But they’re both documented as being runtime libraries… Is there any plan to have non-runtime-library code here? My strong impression was “no”, but you certainly would have more context! =]

If this is all going to be part of a collection of runtime libraries, and the offload stuff just hasn’t been fully integrated, I would like to make a suggestion (which has some relevance here, but we might want to discuss it elsewhere in more detail) that you (or someone) re-organize the directories along the lines of:

…/openmp/runtime/… → …/openmp/…
…/openmp/offload/src/… → …/openmp/src/offload/…
…/openmp/offload/doc/… → …/openmp/doc/offload/…
…/openmp/offload/README.txt → …/openmp/src/offload/README.txt

Would that make sense? As a relative newcomer to the library, it makes more sense to me,and so it might be a bit better at pulling people in, but I dunno if it just doesn’t fit the larger organization.

  1. config-ix.cmake added – compiler flag checks, linker flag checks, some feature checks, threading-model check. This means cmake/${CMAKE_C_COMPILER_ID}/*Flags.cmake directories/files are deleted.

  2. All files inside cmake/ subdirectory are renamed to Libompxyz.cmake

  3. I cut off most of the vestigial parts that were a part of the translation of build.pl system. There are still a few that remain like LibompExports.cmake (copying things to exports/ subdirectory)

I think it might be worth discussing with the broader community if there is any precedent here, especially on other OSes. There might be others in the community with suggestions about how this should work. It’s really hard for me to make suggestions because currently the comments in the BuildPLRules.cmake (haven’t dug into your new patch fully yet) are in terms of ICC and compiling products that aren’t in LLVM’s upstream repository at all.

If this is solely to support building out-of-tree products around the runtime, I think you may end up needing to pull specific bits of it up into your out-of-tree code, as it doesn’t make a lot of sense as is for the upstream folks.

and LibompMicroTests.cmake (small tests to run on the just created library. This is invoked with make libomp-micro-tests), but these can easily be cut off because they are in their own files and surrounded by an if() guard in the CMakeLists.txt file.

  1. Added LLVM-architecture detection if we are part of an LLVM build.

  2. If you want, you can use LIBOMP_ARCH=i386 or LIBOMP_ARCH=x86_64 instead of LIBOMP_ARCH=32 or LIBOMP_ARCH=32e (although 32 and 32e still exist)

  3. Unused functions/macros have been cut away.

Again, thanks for working on this. I’ll try to dig into the new cmake code itself next.

Hello everyone,

I have been working on overhauling the CMake build system for the LLVM
OpenMP Runtime library. Instead of sending 1000 small updates, I decided to
do a complete refactoring/reorganizing/renaming/you-name-it-I-did-it. **All
the interface variables are still the same (LIBOMP_OS, LIBOMP_ARCH, etc.)**.
So there was no change there. Here are the major changes:

1) Renaming internal variables/macros/functions – everything has libomp_ or
LIBOMP_ prefixed to it unless maybe if it is a local variable/parameter
inside a function/macro.

2) config-ix.cmake added – compiler flag checks, linker flag checks, some
feature checks, threading-model check. This means
cmake/${CMAKE_C_COMPILER_ID}/*Flags.cmake directories/files are deleted.

I am strongly apposed to this change. This was designed specifically
to ensure that each set of compilers can easily set their own specific
flags. Do not remove this feature.

3) All files inside cmake/ subdirectory are renamed to Libompxyz.cmake

Why - who cares? fwiw - most cmake files are named CMakeLists.txt

.cmake is typically used for cmake files which are meant to be reused
as "modules".

I am a bit annoyed with the history of the cmake build system. My
original system worked and was removed without much justification
"just because". Now it's being refactored again and it's unclear to me
the driving motivation.

At a quick glance - did you remove support for FreeBSD?

In regards to Chandlers questions

liboffload absolute does not belong inside libomp. Any reference to it
is likely Intel legacy. They should work together, but liboffload is
something which IMHO should be used by other programming models and
not tightly coupled only to OMP.

(OpenCL, C++AMP, CUDA. etc)

I’d like to see generic offloading and device runtime libraries in LLVM as well, but a) they don’t belong in the ‘openmp’ project, and b) this doesn’t seem to be it (at least in its current form).

This really looks like a specific implementation of the OpenMP 4.0 offloading runtime support. the ‘offload.h’ header defines a wide range of OpenMP APIs for offloading and only reserved names for a a very small number of generic offloading interfaces.

There is probably some really good generic offloading logic inside this that would be good to extract into a common accelerator / offloading runtime library though. But probably into a separate runtime project from this one, and there would still probably be an openmp offloading runtime library to satisfy the 4.0 spec stuff.

Maybe I’ve misunderstood the contents of the library?
-Chandler

Chandler, Chris,

Could you have a look at my proposal for OpenMP offloading at
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2015-April/042640.html

There's only one picture that explains the position of liboffload very
well: a box named "Phi offload RTL" is exactly the liboffload we're
discussing. In this proposal the OpenMP-specific bookkeeping of
offloaded data, code and pointers is done inside libomptarget.so, so
the liboffload is abstract from this point of view. Historical
background: originally liboffload was a proprietary interface of Intel
compiler (and it remains to be).

I see the following points here:
- liboffload is naturally not a part of OpenMP infrastructure, rather
lower level interface for the offloading
- liboffload that is discussed supports Intel MIC platform only and
can't be easily reused for other targets
- testing of the liboffload can be done using LIT tests, although
easiest way to obtain them is through the OpenMP sources
- I am not sure if proposed offload RTL interface suits all mentioned
programming model and asking for your help here - we can update the
interface to be as versatile as possible

So contrary to Chandler's proposal I believe there should be something like

.../offload/intelmic/src/...
.../offload/intelmic/test/...
.../offload/nvidia/src/...
.../offload/nvidia/test/...

As for the cross-platform part - the libomptarget.so - it has
OpenMP-specific interface provided to compiler and relies on
underlying interface that is defined by the liboffload. So this part
naturally should go somewhere in libomp repo as part of OpenMP 4.0
support with tests placed along with other at
.../tools/clang/test/OpenMP/...

Regards,
Sergos

One question, would it be OK to use Phabricator for the review?

Absolutely.

Have you looked much at the libc++ and/or compiler-rt CMake build?

Yes, Its where I got the idea for LIBOMP_LIBDIR_SUFFIX (libc++) and config-ix.cmake. I’ve looked at the compiler-rt CMake build system, and it seems like more than we need. It creates multiple libraries that have the architecture in the name (something like libclang_rt.msan-x86_64.a). I only want to create one libomp.so file so libc++ seemed the better candidate to look at. I’m still sifting through libc++ and the LLVM cmake build system to see if there are more predefined LLVM_* CMake variables I can use.

In terms of configuration, libomp is based off an old code base and needs more feature checking, which I still need to add, than libc++ which is written in high level C++. I’m not even positive that libc++ changes much based on architecture. Looking more at the structure of the libcxx and compiler-rt cmake/ directory, I could move every Libomp*.cmake file into libomp/cmake/Modules/ subdirectory similar to the other runtime libraries.

My final high level question (which probably just reflects my not knowing the current status) is what the progress is on using the lit test infrastructure?

The progress here is that it is something else I’ll have to rip apart and add, but it needs to be done. First, I just wanted the base CMake build system to be Ok.

If I can ask a few questions myself, When you say lit test infrastructure, you mean having a check-libomp target that will call llvm-lit directly inside the testsuite directory with proper lit flags, correct?

Also, these tests should have RUN: and CHECK: lines in them as per http://llvm.org/docs/TestingGuide.html ?

So, looking at the source code layout, what is the plan around the offload tree?

I think Sergey Ostanevich knows better about this. I know that the offload/ subdirectory is Intel® MIC Architecture specific and the build of it requires one to build a “host” version which targets the platform hosting the MIC card and a “target” version which targets the Intel® MIC Architecture.

I think it might be worth discussing with the broader community if there is any precedent here, especially on other OSes. There might be others in the community with suggestions about how this should work. It’s really hard for me to make suggestions because currently the comments in the BuildPLRules.cmake (haven’t dug into your new patch fully yet) are in terms of ICC and compiling products that aren’t in LLVM’s upstream repository at all.

Are you talking about the config-ix.cmake file in your first couple sentences here? BuildPLRules.cmake was a file created solely to imitate the exact build process of the original build.pl+Makefile build system. It currently doesn’t work right now.

– Johnny

For everyone who wants a Phabricator review and is not on openmp-commits mailing list.

http://reviews.llvm.org/D10656

– Johnny

Jack,

I figured it better if we did not keep the libgomp symlink in there because people will inevitably have their dynamic library search mechanism find our symlink (that they might not know even exists) when they want the the real libgomp.

But, I put the symlink back in the Phabricator review patch and won't remove it until someone asks.

-- Johnny

No, just removed an unnecessary FREEBSD variable.

-- Johnny