switching CLANG_DEFAULT_OPENMP_RUNTIME to libomp

Chandler,
      Now that openmp trunk is producing the desired renamed libomp
shared library by default and a libgomp symlink to it for use by
-fopenmp=libgomp, do you have any remaining objections to switching
CLANG_DEFAULT_OPENMP_RUNTIME from libgomp to libomp?
              Jack
ps As the recent posting in cfe-commits indicates....

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150525/130067.html

the absence of complaints about the previous -fopenmp=libgomp default
may be more due to misconceptions about the level of support for
OpenMP that provides rather than any real desire to use it in place of
the LLVM openmp (which is completely functional).

Chandler,
      Have you found any blockers in current openmp trunk that would
prevent switching the default to libomp?
               Jack

Chandler,
      It has been over 10 days with no response (3 more than you used
to justify reverting the libiomp5 default for -fopenmp). What blockers
remain in current cfe/openmp svn which would prevent the default for
-fopenmp from being switched over to libomp?
               Jack

Sorry, I missed this at first, and have been travelling.

However, several concerns other than the name have already been raised and I’ve not seen any updates on them. Doesn’t mean they haven’t been addressed, but I’d like to hear about the status or have a chance to go verify its status.

Chandler,
      It has been over 10 days with no response (3 more than you used
to justify reverting the libiomp5 default for -fopenmp). What blockers
remain in current cfe/openmp svn which would prevent the default for
-fopenmp from being switched over to libomp?

From my prior email these were the steps:

"1) Reach the point where the openmp runtime library can be checked out
into a normal llvm / clang build tree (into projects/openmp, perhaps) and
it integrates properly into the build and builds successfully on various
systems.
2) Update the clang "getting started" documentation to suggest doing this
if the user wants OpenMP support (change
http://clang.llvm.org/get_started.html to say what to check out and where
-- no steps other than an 'svn co' should be necessary).
3) Change the default for CLANG_DEFAULT_OPENMP_RUNTIME to libomp (possibly
conditioned on a "is libomp part of this build?" test)."

Step 2 has certainly not happened. Has step 1 happened?

Chandler,
      It has been over 10 days with no response (3 more than you used
to justify reverting the libiomp5 default for -fopenmp). What blockers
remain in current cfe/openmp svn which would prevent the default for
-fopenmp from being switched over to libomp?

From my prior email these were the steps:

"1) Reach the point where the openmp runtime library can be checked out into
a normal llvm / clang build tree (into projects/openmp, perhaps) and it
integrates properly into the build and builds successfully on various
systems.
2) Update the clang "getting started" documentation to suggest doing this if
the user wants OpenMP support (changehttp://clang.llvm.org/get_started.html
to say what to check out and where -- no steps other than an 'svn co' should
be necessary).
3) Change the default for CLANG_DEFAULT_OPENMP_RUNTIME to libomp (possibly
conditioned on a "is libomp part of this build?" test)."

Step 2 has certainly not happened. Has step 1 happened?

I have been building current openmp in the llvm/projects/openmp
location as a cmake
build on x86_64-apple-darwin14 daily without issues. So step 1 is
complete on darwin.

>>
>> Chandler,
>> It has been over 10 days with no response (3 more than you used
>> to justify reverting the libiomp5 default for -fopenmp). What blockers
>> remain in current cfe/openmp svn which would prevent the default for
>> -fopenmp from being switched over to libomp?
>
>
> From my prior email these were the steps:
>
> "1) Reach the point where the openmp runtime library can be checked out
into
> a normal llvm / clang build tree (into projects/openmp, perhaps) and it
> integrates properly into the build and builds successfully on various
> systems.
> 2) Update the clang "getting started" documentation to suggest doing
this if
> the user wants OpenMP support (changehttp://
clang.llvm.org/get_started.html
> to say what to check out and where -- no steps other than an 'svn co'
should
> be necessary).
> 3) Change the default for CLANG_DEFAULT_OPENMP_RUNTIME to libomp
(possibly
> conditioned on a "is libomp part of this build?" test)."
>
> Step 2 has certainly not happened. Has step 1 happened?

I have been building current openmp in the llvm/projects/openmp
location as a cmake
build on x86_64-apple-darwin14 daily without issues. So step 1 is
complete on darwin.

If you can let me know what I should check out where, I'll be happy to test
it on Linux for you. (Should I check out
http://llvm.org/svn/llvm-project/openmp/trunk/ or just the runtime/
subdirectory there?)

It looks like the openmp directory is not listed in llvm's
projects/CMakeLists.txt, so there may be some more integration work
required for this to integrate properly into the llvm/clang build.

Richard,
      I do my local build using tarballs generated from current trunk
svn pulls. The rough formula I use (after these are extracted) are...

cd llvm-3.7.0.src
mv ../cfe-3.7.0.src tools/clang
mv ../compiler-rt-3.7.0.src projects/compiler-rt
mv ../libcxx-3.7.0.src projects/libcxx
mv ../openmp-3.7.0.src projects/openmp
mv ../polly-3.7.0.src tools/polly
mv ../clang-tools-extra-3.7.0.src tools/clang/tools/extra
mkdir build
cd build
cmake -DLLVM_BUILD_32_BITS:BOOL=OFF -DLLVM_TARGETS_TO_BUILD=X86 \
            -DLLVM_ENABLE_ASSERTIONS=OFF -DCMAKE_BUILD_TYPE=Release
-DLIBOMP_OSX_ARCHITECTURES="x86_64;i386" \
            -DCMAKE_OSX_SYSROOT:STRING=/
-DCMAKE_OSX_DEPLOYMENT_TARGET:STRING="" \
            -DLLVM_ENABLE_LIBCXX=ON -DLIBCXX_LIBCPPABI_VERSION=""
-DENABLE_CLANG_OPENMP=ON ..
make VERBOSE=1

This builds the libomp.dylib as a fat binary on x86_64 darwin (for
both i386/x86_64 support).
I always execute "perl -pi -e 's|libgomp|libomp|g' CMakeLists.txt" in
tools/clang before the build to
switch the -fopenmp default to libomp.
             Jack

I’ve had time to check, and a large amount of my concerns over the openmp CMake build have not been addressed.

For example, it defines at global scope a reasonably large number of variables like “os”, “arch” and friends.

Another example is that it uses a completely novel way of managing compiler flags compared to all of LLVM, Clang, CompilerRT, libc++, and libc++abi. I have to think that even if some of these don’t apply, at least one does. In particular, the openmp runtime seems to have almost identical needs for configuration as the libc++ runtime library, with the possible exception of needing to run an assembler.

I also don’t see any way to run any of the OpenMP tests using the CMake build, but maybe I’m just missing it.

I’ll repeat what I said previously in the other thread: I think that the OpenMP CMake build should be replaced with one that functions essentially the same way as the libc++ CMake build, including the use of ‘lit’ to drive the test suite. libc++ has extremely similar build configuration and testsuite needs, and so I feel like we shouldn’t have two radically different ways of doing everything within the same project. It makes the maintenance burden for those of us that work on the larger LLVM project’s CMake support much, much higher. Perhaps a detailed explanation of why that isn’t feasible or reasonable?

I've had time to check, and a large amount of my concerns over the openmp
CMake build have not been addressed.

For example, it defines at global scope a reasonably large number of
variables like "os", "arch" and friends.

Chandler,
       This is likely residue from the conversion of the original
build.pl-based build
system to the cmake build system. My understanding is that the cmake
build system
still uses some of the .pl scripts from the legacy build.pl build
system. So some of
the renaming may have to be postponed until the openmp cmake system is totally
purged of any remnants of the old build.pl system.

Another example is that it uses a completely novel way of managing compiler
flags compared to all of LLVM, Clang, CompilerRT, libc++, and libc++abi. I
have to think that even if some of these don't apply, at least one does. In
particular, the openmp runtime seems to have almost identical needs for
configuration as the libc++ runtime library, with the possible exception of
needing to run an assembler.

Again some of this may be a fall-out of the migration of the build system from
build.pl to cmake. It might help if you listed specific examples of
the flags you
wanted to be standardized.

I also don't see any way to run any of the OpenMP tests using the CMake
build, but maybe I'm just missing it.

The cmake support for running the test suite is in fact missing but shouldn't
be hard for Jeremy to implement.
                  Jack

For example, it defines at global scope a reasonably large number of
variables like "os", "arch" and friends.

Can you please point out what you are talking about here. I have prefixed all the CACHED variables with LIBOMP_. These variables are now LIBOMP_OS, LIBOMP_ARCH and friends.

Another example is that it uses a completely novel way of managing
compiler flags compared to all of LLVM, Clang, CompilerRT, libc++, and
libc++abi. I have to think that even if some of these don't apply, at
least one does. In particular, the openmp runtime seems to have almost
identical needs for configuration as the libc++ runtime library, with
the possible exception of needing to run an assembler.

Yes, I have been working on cmake/config-ix.cmake for libomp, it is almost complete. But I completely agree this has to conform to the conventional LLVM style of having cmake/config-ix.cmake do compiler flag checks, library checks, feature checks, etc.

-- Johnny

From the posting of the 3.7svn release schedule at...

http://lists.cs.uiuc.edu/pipermail/cfe-dev/2015-June/043595.html

it really seems like the time has arrived to decide if the new
-fopenmp=libomp support will be the default for the 3.7 release.
            Jack
ps IMHO as an end-user, it seems pretty idiotic to disable it in favor
of a completely non-op version of -fopenmp=libgomp.

   From the posting of the 3.7svn release schedule at...

http://lists.cs.uiuc.edu/pipermail/cfe-dev/2015-June/043595.html

it really seems like the time has arrived to decide if the new
-fopenmp=libomp support will be the default for the 3.7 release.
            Jack
ps IMHO as an end-user, it seems pretty idiotic to disable it in favor
of a completely non-op version of -fopenmp=libgomp.

Also from a PR-viewpoint, it is going to be rather hard to explain why
a fully functional OpenMP-3.1 libomp was shunted
aside in favor of the non-functional libgomp support.

For example, it defines at global scope a reasonably large number of
variables like “os”, “arch” and friends.

Can you please point out what you are talking about here. I have prefixed all the CACHED variables with LIBOMP_. These variables are now LIBOMP_OS, LIBOMP_ARCH and friends.

When I re-testing things, I still saw some things in the cache. Not sure if that was staleness or on-going issues.

Anyways, I suspect the most productive thing is to focus on your re-work patch which I’ll look at. As I said before, I strongly believe that is the best path forward.

From the posting of the 3.7svn release schedule at…

http://lists.cs.uiuc.edu/pipermail/cfe-dev/2015-June/043595.html

it really seems like the time has arrived to decide if the new
-fopenmp=libomp support will be the default for the 3.7 release.
Jack

Our releases are time based, not feature based. And we shouldn’t turn on features until they’re ready, regardless of whether it’s convenient to the release schedule.

ps IMHO as an end-user, it seems pretty idiotic to disable it in favor
of a completely non-op version of -fopenmp=libgomp.

You seem to ignore the fact that there are lots of other end-users, some of which are represented by myself or others, and some of which have and continue to use libgomp as a functional if no-op implementation. =/

I don’t think characterizing this as “idiotic” is productive at all. I think it would be much more productive to help contribute patches to improve things than to criticize those who do. =/ I’m personally going to focus on reviewing Jonathan’s patches instead of continuing this thread.

   From the posting of the 3.7svn release schedule at...

http://lists.cs.uiuc.edu/pipermail/cfe-dev/2015-June/043595.html

it really seems like the time has arrived to decide if the new
-fopenmp=libomp support will be the default for the 3.7 release.
            Jack

Our releases are time based, not feature based. And we shouldn't turn on
features until they're ready, regardless of whether it's convenient to the
release schedule.

ps IMHO as an end-user, it seems pretty idiotic to disable it in favor
of a completely non-op version of -fopenmp=libgomp.

You seem to ignore the fact that there are lots of other end-users, some of
which are represented by myself or others, and some of which have and
continue to use libgomp as a functional if no-op implementation. =/

Why is is acceptable to force those end-users who want to use the libomp
support to invoke the compiler with -fopenmp=libomp and yet you seem to
find it an unacceptable burden for your own users to be forced to invoke the
compiler with -fopenmp=libgomp. Certainly they are capable of adding that
to their build flags for themselves.
              Jack
ps Current openmp trunk installs a libgomp symlink to libomp which allows
-fopenmp=libgomp to work seamlessly (at least in my testing against
an external copy of the OpenMP3.1_Validation test suite).
           Jack

I completely agree. Given that -fopenmp=libomp means ‘and please make OpenMP actually work’, there’s no good reason for -fopenmp=libgomp (which means ‘and please emit OpenMP programs that don’t link correctly if libgomp.so is not a symlink to libomp.so’) to be the default.

David

Richard,
      I do my local build using tarballs generated from current trunk
svn pulls. The rough formula I use (after these are extracted) are...

cd llvm-3.7.0.src
mv ../cfe-3.7.0.src tools/clang
mv ../compiler-rt-3.7.0.src projects/compiler-rt
mv ../libcxx-3.7.0.src projects/libcxx
mv ../openmp-3.7.0.src projects/openmp

You will probably need to explicit support for checking out openmp here to
llvm/projects/CMakeLists.txt, in order to get the "same as libcxx" behavior
that Chandler is requesting.

mv ../polly-3.7.0.src tools/polly
mv ../clang-tools-extra-3.7.0.src tools/clang/tools/extra
mkdir build
cd build
cmake -DLLVM_BUILD_32_BITS:BOOL=OFF -DLLVM_TARGETS_TO_BUILD=X86 \
            -DLLVM_ENABLE_ASSERTIONS=OFF -DCMAKE_BUILD_TYPE=Release
-DLIBOMP_OSX_ARCHITECTURES="x86_64;i386" \
            -DCMAKE_OSX_SYSROOT:STRING=/
-DCMAKE_OSX_DEPLOYMENT_TARGET:STRING="" \
            -DLLVM_ENABLE_LIBCXX=ON -DLIBCXX_LIBCPPABI_VERSION=""
-DENABLE_CLANG_OPENMP=ON ..
make VERBOSE=1

As promised, I tried this out on a Linux machine. Here's what I did:

* check out all the pieces, including putting
http://llvm.org/svn/llvm-project/openmp/trunk/ into llvm/projects/openmp
* run cmake without specifying any special flags (this should "just work")
* build
* run clang from build area and test

Here's what I got:

# From cmake:

CMake Warning (dev) at projects/openmp/runtime/src/CMakeLists.txt:26
(add_custom_command):
  Policy CMP0040 is not set: The target in the TARGET signature of
  add_custom_command() must exist. Run "cmake --help-policy CMP0040" for
  policy details. Use the cmake_policy command to set the policy and
  suppress this warning.

  The target name "common" is unknown in this context.

It looks like the cmake build is not correctly configured here. I'm not
sure whether these custom commands are the right way to handle these header
files, but the TARGET should presumably be "libomp-common" or "libomp-mod",
not just "common" or "mod". I assume this will be fixed by the cmake build
system rework.

# From build:

*Lots* of warnings, but a successful build. These warnings should be fixed
(or, where appropriate, suppressed), but I don't personally think that
needs to be a prerequisite for changing the default value of -fopenmp=.

# From running Clang with -fopenmp=libomp:

Everything I tested works fine (the not-yet-installed library is not found,
but that seems like the right behaviour if we expect this library to live
in /usr/lib rather than in a compiler-specific library area). However, I
couldn't find a make target to run the libomp tests, and I couldn't find a
target to install the libomp runtime.

This builds the libomp.dylib as a fat binary on x86_64 darwin (for

Richard,
      Please try linux again with current openmp trunk and the
proposed cmake overhaul changes from...

http://reviews.llvm.org/D10656

using the patch at...

http://reviews.llvm.org/file/data/yelxztueytythwycxwjx/PHID-FILE-m5boinw4uugif7ky6tpq/D10656.diff

applied.

Done. The CMP0040 is gone, and I still get a working libomp.so. However,
some of the libomp-micro-tests checks don't pass:

libomp-test-touch fails because it sets LD_LIBRARY_PATH to include
[...]/build/projects/openmp/runtime/src, rather than the correct path
[...]/build/lib

libomp-test-instr fails with "check-instruction-set.pl: (x) Only works on
lin_32 and lin_mic platforms."