This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Disable building libomptarget and add CMake switch
ClosedPublic

Authored by Hahnfeld on Jul 20 2017, 11:42 PM.

Details

Summary

Introduce OPENMP_ENABLE_LIBOMPTARGET which defaults to OFF at the moment.

libomptarget is not yet ready for prime time:

  • Offloading to NVIDIA GPUs is not completed yet (compiler, device RTL)
  • The generic ELF plugin for offloading to the host (meant for testing) uses a single instance of the OpenMP runtime (libomp). That is why omp_is_initial_device() returns 1 which makes the tests fail.

Because of these reasons, we want to disable building (and testing!) for release 5.0.

See https://bugs.llvm.org/show_bug.cgi?id=33859

Diff Detail

Repository
rL LLVM

Event Timeline

Hahnfeld created this revision.Jul 20 2017, 11:42 PM
hans added a subscriber: hans.Jul 21 2017, 12:03 AM
hans added inline comments.
CMakeLists.txt
8 ↗(On Diff #107634)

Since the tests fail by default, I think it should be disabled in the build by default.

trunk should always be in a working state.

Hahnfeld added inline comments.Jul 21 2017, 1:07 AM
CMakeLists.txt
8 ↗(On Diff #107634)

Well, it's broken since January so nobody cared enough. @gtbercea, what do you think?

hans added inline comments.Jul 21 2017, 1:10 AM
CMakeLists.txt
8 ↗(On Diff #107634)

Most people don't check out or build the openmp repo, and your buildbot doesn't run the tests -- which is another reason the code shouldn't be built by default.

Hahnfeld updated this revision to Diff 107849.Jul 23 2017, 11:15 PM
Hahnfeld retitled this revision from [CMake] Allow to disable building libomptarget to [CMake] Disable building libomptarget and add CMake switch.
Hahnfeld edited the summary of this revision. (Show Details)

Disable building by default.

Hahnfeld marked 3 inline comments as done.Jul 23 2017, 11:17 PM
Hahnfeld added inline comments.
CMakeLists.txt
8 ↗(On Diff #107634)

Updated the patch. I'll let it sit for two days or so to give the others time to respond.

hans accepted this revision.Jul 25 2017, 10:26 AM

Looks good to me. Thanks!

This revision is now accepted and ready to land.Jul 25 2017, 10:26 AM
This revision was automatically updated to reflect the committed changes.
Hahnfeld marked an inline comment as done.

I've committed the patch without an explicit answer from the IBM guys to unblock the release

hans added a comment.Jul 26 2017, 8:47 AM

I've committed the patch without an explicit answer from the IBM guys to unblock the release

Thanks! Merged to 5.0 in r309126.