[Openmp-dev] CMake overhaul

[Chandler Carruth]

On Mon, Jun 22, 2015 at 2:26 PM Peyton, Jonathan L <
jonathan.l.peyton at intel.com> wrote:

>  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.

>  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)
>
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.
>
> 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.
>
Again, thanks for working on this. I'll try to dig into the new cmake code
itself next.

>
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/openmp-dev/attachments/20150623/cab91059/attachment.html>