Page MenuHomePhabricator

[OpenMP] Disabled profiling in `libomp` by default to unblock link errors
ClosedPublic

Authored by tianshilei1992 on Jan 27 2021, 8:12 PM.

Details

Summary

Link error occurred when time profiling in libomp is enabled by default
because libomp is assumed to be a C library but the dependence on
libLLVMSupport for profiling is a C++ library. Currently the issue blocks all
OpenMP tests in Phabricator.

This patch set a new CMake option OPENMP_ENABLE_LIBOMP_PROFILING to
enable/disable the feature. By default it is disabled. Note that once time
profiling is enabled for libomp, it becomes a C++ library.

Diff Detail

Event Timeline

tianshilei1992 created this revision.Jan 27 2021, 8:12 PM
tianshilei1992 requested review of this revision.Jan 27 2021, 8:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 27 2021, 8:12 PM
jdoerfert accepted this revision.Jan 27 2021, 8:17 PM

LG, we need to backport this too. And we need to document that profiling multi-threaded applications needs this flag in the build.

openmp/docs/design/Runtimes.rst
1

changes in this file seem unrelated.

This revision is now accepted and ready to land.Jan 27 2021, 8:17 PM
jdoerfert requested changes to this revision.Jan 27 2021, 8:18 PM

LG, thanks

openmp/docs/design/Runtimes.rst
54

to -> into

This revision now requires changes to proceed.Jan 27 2021, 8:18 PM
jdoerfert accepted this revision.Jan 27 2021, 8:18 PM

wrong selection :(

This revision is now accepted and ready to land.Jan 27 2021, 8:18 PM
tianshilei1992 marked 2 inline comments as done.Jan 27 2021, 8:22 PM
This revision was landed with ongoing or failed builds.Jan 28 2021, 4:24 AM
This revision was automatically updated to reflect the committed changes.

The cherrypick of this commit to the 12.x branch broke building of openmp there, with errors like these:

CMake Error at CMakeLists.txt:93 (add_subdirectory):
  The binary directory

    /build/llvm-project/openmp/build-i686/runtime

  is already used to build a source directory.  It cannot be used to build
  source directory

    /build/llvm-project/openmp/runtime

  Specify a unique binary directory name.

That's because on the main branch, D95398 moved the add_subdirectory(runtime), but that bit isn't cherrypicked to the release branch, and the cherry pick conflict seems to have been resolved incorrectly.

The cherrypick of this commit to the 12.x branch broke building of openmp there, with errors like these:

CMake Error at CMakeLists.txt:93 (add_subdirectory):
  The binary directory

    /build/llvm-project/openmp/build-i686/runtime

  is already used to build a source directory.  It cannot be used to build
  source directory

    /build/llvm-project/openmp/runtime

  Specify a unique binary directory name.

That's because on the main branch, D95398 moved the add_subdirectory(runtime), but that bit isn't cherrypicked to the release branch, and the cherry pick conflict seems to have been resolved incorrectly.

Thanks for the report. Now it is fixed.