[libomptarget] Introduce LIBOMPTARGET_ENABLE_DEBUG cmake option.

[Meinersbur]

Summary

At the moment, support for runtime debug output using the OMPTARGET_DEBUG=1 environment variable is only available with CMAKE_BUILD_TYPE=Debug builds. The patch allows setting it independently using the LIBOMPTARGET_ENABLE_DEBUG option, which is enabled by default depending on CMAKE_BUILD_TYPE. That is, unless this option is set explicitly, nothing changes. This is the same mechanism used by LLVM for LLVM_ENABLE_ASSERTIONS.

This patch also remove adding -g -O0 in debug builds, it should be handled by cmake.

Idea by @hfinkel

[grokos]

I'm not sure what we are trying to do here. OMPTARGET_DEBUG is not an environment variable, it's a preprocessor definition. It is used by libomptarget/include/omptarget.h to include all necessary header files and define the DEBUGP macro appropriately, which in turn is used by libomptarget/src/private.h to define the DP macro used throughout the library.

The env var you are referring to is LIBOMPTARGET_DEBUG and it switches debug output on and off, provided that the library has been compiled with -DOMPTARGET_DEBUG so that the appropriate macros have been defined in omptarget.h and private.h.

If you want to make debug output available in release mode via a new LIBOMPTARGET_ENABLE_DEBUG cmake option, then you should add another check like:

if(LIBOMPTARGET_ENABLE_DEBUG)
  add_definitions(-DOMPTARGET_DEBUG)
endif()

libomptarget/CMakeLists.txt

48 ↗

(On Diff #179136)

This must stay, it's a preprocessor definition, not an env var.

[Meinersbur]

• Define OMPTARGET_DEBUG preprocessor symbol if LIBOMPTARGET_ENABLE_DEBUG
• Fix regression test execution

[Meinersbur]

Sorry for the delay. I missed your reply during the holidays.

Thank you for the hint. Somewhat confusingly LIBOMPTARGET_DEBUG is a preprocessor symbol, a CMake config variable and an environment variable. I fixed the execution of check-libomptarget on my machine (it replaced all commands in RUN with ignored-command) so I can confirm that it works.

[Meinersbur]

• Undo whitespace change

[Meinersbur]

ping

[grokos]

This revision looks much better. Please have a look at the inline comment.

libomptarget/CMakeLists.txt

48–49 ↗

(On Diff #190186)

Why are these lines removed? When building in debug mode we need to have debug symbols (-g) and prevent the compiler from optimizing things away (-O0) in order to be able to use a debugger correctly.

[Meinersbur]

libomptarget/CMakeLists.txt

48–49 ↗

(On Diff #190186)

This is mentioned in the summary.

This patch also remove adding -g -O0 in debug builds, it should be handled by cmake.

With CMAKE_BUILD_TYPE=Debug, CMake will add CMAKE_{C|CXX}_FLAGS_DEBUG to CMAKE_{C|CXX}_FLAGS. CMAKE_{C|CXX}_FLAGS_DEBUG is preinitialized with the compiler's debug options, e.g. -O0 -g for gcc. Adding these flags using add_definitions is either redundant, might result in an error for non-gcc-like command line interfaces, or clashes with user-provided debug flags (such as cmake -DCMAKE_C_FLAGS_DEBUG="-gdwarf=5 -Og").

Unless I am overseeing something, in which case please point me to it.

[grokos]

OK, now it makes sense. I had forgotten what was in the description since the patch was first submitted. :)

Looks good now.

[Meinersbur]

@grokos Thank you for the review!

openmp/trunk/libomptarget/CMakeLists.txt

 set (LIBOMPTARGET_ALL_TARGETS "${LIBOMPTARGET_ALL_TARGETS} powerpc64-ibm-linux-gnu")
 set (LIBOMPTARGET_ALL_TARGETS "${LIBOMPTARGET_ALL_TARGETS} x86_64-pc-linux-gnu")
 set (LIBOMPTARGET_ALL_TARGETS "${LIBOMPTARGET_ALL_TARGETS} nvptx64-nvidia-cuda")
 
 # Once the plugins for the different targets are validated, they will be added to
 # the list of supported targets in the current system.
 set (LIBOMPTARGET_SYSTEM_TARGETS "")
 
-# If building this library in debug mode, we define a macro to enable
-# dumping progress messages at runtime.
+# Check whether using debug mode. In debug mode, allow dumping progress
+# messages at runtime by default. Otherwise, it can be enabled
+# independently using the LIBOMPTARGET_ENABLE_DEBUG option.
 string( TOLOWER "${CMAKE_BUILD_TYPE}" LIBOMPTARGET_CMAKE_BUILD_TYPE)
 if(LIBOMPTARGET_CMAKE_BUILD_TYPE MATCHES debug)
+  option(LIBOMPTARGET_ENABLE_DEBUG "Allow debug output with the environment variable LIBOMPTARGET_DEBUG=1" ON)
+else()
+  option(LIBOMPTARGET_ENABLE_DEBUG "Allow debug output with the environment variable LIBOMPTARGET_DEBUG=1" OFF)
+endif()
+if(LIBOMPTARGET_ENABLE_DEBUG)
   add_definitions(-DOMPTARGET_DEBUG)
-  add_definitions(-g)
-  add_definitions(-O0)
 endif()
 
 include_directories(include)
 
 # Build target agnostic offloading library.
 add_subdirectory(src)
 
 # Retrieve the path to the resulting library so that it can be used for 

openmp/trunk/libomptarget/test/CMakeLists.txt

 # CMakeLists.txt file for unit testing OpenMP offloading runtime library.
 if(NOT OPENMP_TEST_COMPILER_ID STREQUAL "Clang" OR
    OPENMP_TEST_COMPILER_VERSION VERSION_LESS 6.0.0)
   libomptarget_say("Can only test with Clang compiler in version 6.0.0 or later.")
   libomptarget_warning_say("The check-libomptarget target will not be available!")
   return()
 endif()
 
-if(LIBOMPTARGET_CMAKE_BUILD_TYPE MATCHES debug)
+if(LIBOMPTARGET_ENABLE_DEBUG)
   set(LIBOMPTARGET_DEBUG True)
 else()
   set(LIBOMPTARGET_DEBUG False)
 endif()
 
 add_openmp_testsuite(check-libomptarget "Running libomptarget tests" ${CMAKE_CURRENT_BINARY_DIR} DEPENDS omptarget omp)
 
 # Configure the lit.site.cfg.in file
 set(AUTO_GEN_COMMENT "## Autogenerated by libomptarget configuration.\n# Do not edit!")
 configure_file(lit.site.cfg.in lit.site.cfg @ONLY)