[openmp] Disable archer if LIBOMP_OMPT_SUPPORT is off

[mgorny]

Summary

This fixed build failures due to missing ompt headers.

See https://bugs.gentoo.org/700762.

[jdoerfert]

Seems reasonable, @protze.joachim agreed?

[Hahnfeld]

I'm not sure LIBOMP_OMPT_SUPPORT is guaranteed to be there yet when defined in runtime/. Maybe it would be better to move the option to the top-level openmp/CMakeList.txt and rename it to OPENMP_OMPT_SUPPORT (with compatibility support for LIBOMP_OMPT_SUPPORT)

[mgorny]

In D73249#1835526, @Hahnfeld wrote:

I'm not sure LIBOMP_OMPT_SUPPORT is guaranteed to be there yet when defined in runtime/.

If by 'guaranteed' you mean in the current code, then I've tested this patch with it being both true and false.

Maybe it would be better to move the option to the top-level openmp/CMakeList.txt and rename it to OPENMP_OMPT_SUPPORT (with compatibility support for LIBOMP_OMPT_SUPPORT)

I don't see a problem moving it. However, I'm not sure if rename is really desired. After all, AFAIU it's a characteristic of the library.

[Hahnfeld]

In D73249#1835551, @mgorny wrote:

In D73249#1835526, @Hahnfeld wrote:

I'm not sure LIBOMP_OMPT_SUPPORT is guaranteed to be there yet when defined in runtime/.

If by 'guaranteed' you mean in the current code, then I've tested this patch with it being both true and false.

Ok, as long as it works for the release (that's probably the motivation?) I'm fine with this.

Maybe it would be better to move the option to the top-level openmp/CMakeList.txt and rename it to OPENMP_OMPT_SUPPORT (with compatibility support for LIBOMP_OMPT_SUPPORT)

I don't see a problem moving it. However, I'm not sure if rename is really desired. After all, AFAIU it's a characteristic of the library.

Currently yes, but OMPT callbacks might also be added to libomptarget. We'll need to move then at the latest, but I agree that it might be out-of-scope for this patch (I was just thinking for solutions if LIBOMP_OMPT_SUPPORT did not work here).

[mgorny]

Yes, indeed the goal is to unbreak 10.x. I'd also agree to making bigger changes in 11.x separately.

[jdoerfert]

@Hahnfeld can you accept, assuming you are happy with this as a fix for now?

[protze.joachim]

LGTM

[protze.joachim]

In D73249#1835526, @Hahnfeld wrote:

I'm not sure LIBOMP_OMPT_SUPPORT is guaranteed to be there yet when defined in runtime/. Maybe it would be better to move the option to the top-level openmp/CMakeList.txt and rename it to OPENMP_OMPT_SUPPORT (with compatibility support for LIBOMP_OMPT_SUPPORT)

From my understanding of cmake scoping, this variable is visible because it's a cached option?

[mgorny]

In D73249#1836528, @protze.joachim wrote:

From my understanding of cmake scoping, this variable is visible because it's a cached option?

Yes. It is also important that runtime directory is added before tools, so the default is set before it is checked.

openmp/tools/archer/CMakeLists.txt

 # //===----------------------------------------------------------------------===//
 # //
 # // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 # // See https://llvm.org/LICENSE.txt for details.
 # // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 # //
 # //===----------------------------------------------------------------------===//
   
   
 
+if(LIBOMP_OMPT_SUPPORT)
   include_directories(${LIBOMP_INCLUDE_DIR})
 
   add_library(archer SHARED ompt-tsan.cpp)
   add_library(archer_static STATIC ompt-tsan.cpp)
 
   install(TARGETS archer archer_static
     LIBRARY DESTINATION ${OPENMP_INSTALL_LIBDIR}
     ARCHIVE DESTINATION ${OPENMP_INSTALL_LIBDIR})
 
   add_subdirectory(tests)
+endif()