[Flang-RT][Offload] Always use LLVM-built GTest

[Meinersbur]

The Offload and Flang-RT had the ability to compile GTest themselves. But in bootstrapping builds, LLVM_LIBRARY_OUTPUT_INTDIR points to the same location as the stage1 build. If both are building GTest, they everwrite each others libllvm_gtest.a and libllvm_test_main.a which causes #143134.

This PR removes the ability for the Offload/Flang-RT runtimes to build their own GTest and instead relies on the stage1 build of GTest. This was already the case with LLVM_INSTALL_GTEST=ON configurations. For LLVM_INSTALL_GTEST=OFF configurations, we now also export gtest into the buildtree configuration. Ultimately, this reduces combinatorial explosion in which unittests could be built (LLVM_INSTALL_GTEST=ON, GTest built by Offload, GTest built by Flang-RT, GTest built by Offload and also used by Flang-RT).

GTest and therefore Offload/Runtime unittests will not be available if the runtimes are configured against an LLVM install tree. Since llvm-lit isn't available in the install tree either, it doesn't matter.

Note that compiler-rt and libc also use GTest in non-default configrations. libc also depends on LLVM's GTest build (and would error-out if unavailable), but compiler-rt builds it completely different.

Fixes #143134

[DavidSpickett]

Thanks for working on this, I wouldn't have had a clue how to fix this.

[DavidSpickett]

I presume you'll get someone offload related to review later.

The errors we saw on the bots I think are possible with offload too, but I've no hard evidence for that. Keeping them all consistent is good anyway.

[llvmbot]

@llvm/pr-subscribers-flang-runtime

@llvm/pr-subscribers-offload

Author: Michael Kruse (Meinersbur)

Changes

The Offload and Flang-RT had the ability to compile GTest themselves. But in bootstrapping builds, LLVM_LIBRARY_OUTPUT_INTDIR points to the same location as the stage1 build. If both are building GTest, they everwrite each others libllvm_gtest.a and libllvm_test_main.a which causes #143134.

This PR removes the ability for the Offload/Flang-RT runtimes to build their own GTest and instead relies on the stage1 build of GTest. This was already the case with LLVM_INSTALL_GTEST=ON configurations. For LLVM_INSTALL_GTEST=OFF configurations, we now also export gtest into the buildtree configuration. Ultimately, this reduces combinatorial explosion in which unittests could be built (LLVM_INSTALL_GTEST=ON, GTest built by Offload, GTest built by Flang-RT, GTest built by Offload and also used by Flang-RT).

GTest and therefore Offload/Runtime unittests will not be available if the runtimes are configured against an LLVM install tree. Since llvm-lit isn't available in the install tree either, it doesn't matter.

Note that compiler-rt and libc also use GTest in non-default configrations. libc also depends on LLVM's GTest build (and would error-out if unavailable), but compiler-rt builds it completely different.

Fixes #143134

---

Full diff: https://github.com/llvm/llvm-project/pull/143682.diff

6 Files Affected:

• (modified) flang-rt/CMakeLists.txt (+1-1)
• (modified) flang-rt/test/CMakeLists.txt (+12-16)
• (modified) flang-rt/unittests/CMakeLists.txt (+24-18)
• (modified) offload/CMakeLists.txt (+1-11)
• (modified) offload/unittests/CMakeLists.txt (+14)
• (modified) third-party/unittest/CMakeLists.txt (+14-4)

diff --git a/flang-rt/CMakeLists.txt b/flang-rt/CMakeLists.txt
index b3b6e00f7c0c8..d048ac4b3e5a4 100644
--- a/flang-rt/CMakeLists.txt
+++ b/flang-rt/CMakeLists.txt
@@ -334,8 +334,8 @@ if (LLVM_INCLUDE_EXAMPLES)
 endif ()
 
 if (FLANG_RT_INCLUDE_TESTS)
-  add_subdirectory(unittests)
   add_subdirectory(test)
+  add_subdirectory(unittests)
 else ()
   add_custom_target(check-flang-rt)
 endif()
diff --git a/flang-rt/test/CMakeLists.txt b/flang-rt/test/CMakeLists.txt
index cb48d22d3accc..f10bba388fc4a 100644
--- a/flang-rt/test/CMakeLists.txt
+++ b/flang-rt/test/CMakeLists.txt
@@ -23,28 +23,24 @@ configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.py
 )
 
-if (TARGET FlangRTUnitTests)
-  configure_lit_site_cfg(
-    ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.py.in
-    ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg.py
-    MAIN_CONFIG
-    ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.cfg.py
-  )
+configure_lit_site_cfg(
+  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg.py
+  MAIN_CONFIG
+  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.cfg.py
+)
 
-  configure_lit_site_cfg(
-    ${CMAKE_CURRENT_SOURCE_DIR}/NonGtestUnit/lit.site.cfg.py.in
-    ${CMAKE_CURRENT_BINARY_DIR}/NonGtestUnit/lit.site.cfg.py
-    MAIN_CONFIG
-    ${CMAKE_CURRENT_SOURCE_DIR}/NonGtestUnit/lit.cfg.py
-  )
-endif ()
+configure_lit_site_cfg(
+  ${CMAKE_CURRENT_SOURCE_DIR}/NonGtestUnit/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/NonGtestUnit/lit.site.cfg.py
+  MAIN_CONFIG
+  ${CMAKE_CURRENT_SOURCE_DIR}/NonGtestUnit/lit.cfg.py
+)
 
 
 add_custom_target(flang-rt-test-depends)
 set_target_properties(flang-rt-test-depends PROPERTIES FOLDER "Flang-RT/Meta")
 add_dependencies(flang-rt-test-depends
-    FlangRTUnitTests
-    flang_rt.runtime.unittest
     flang_rt.runtime
   )
 
diff --git a/flang-rt/unittests/CMakeLists.txt b/flang-rt/unittests/CMakeLists.txt
index e2a50d8b40a9d..5282196174134 100644
--- a/flang-rt/unittests/CMakeLists.txt
+++ b/flang-rt/unittests/CMakeLists.txt
@@ -6,33 +6,39 @@
 #
 #===------------------------------------------------------------------------===#
 
+# Target that depends on all unittests
+add_custom_target(FlangRTUnitTests)
+set_target_properties(FlangRTUnitTests PROPERTIES FOLDER "Flang-RT/Meta")
+
 # LLVM uses a modified version of GTest that uses LLVMSupport for console
-# output. Therefore it also needs to include files from LLVM. Unfortunately,
-# LLVM/GTest doesn't add the include search path itself. Limiting the scope
-# using target_include_directories does not work because with
-# LLVM_INSTALL_GTEST=ON, as llvm_gtest is an IMPORT library.
-include_directories("${LLVM_INCLUDE_DIR}" "${LLVM_MAIN_INCLUDE_DIR}")
-
-# Add GTest if not already present.
-# Using a function so LLVM_SUBPROJECT_TITLE does not propagate.
-function (build_gtest)
-  set(LLVM_SUBPROJECT_TITLE "Third-Party/Google Test")
-  add_subdirectory("${LLVM_THIRD_PARTY_DIR}/unittest" "${CMAKE_CURRENT_BINARY_DIR}/third-party/unittest")
-endfunction ()
+# output. We are using the pre-compiled GTest library from the LLVM build,
+# if available. Otherwise, do nothing.
+
+if (CMAKE_CROSSCOMPILING)
+  # TODO: It is possible that LLVM_GTEST_RUN_UNDER defines an emulator or
+  #       ssh remote command invocation; for this case provide an option to
+  #       enable unittests.
+  message(STATUS "Flang-RT unittests disabled because we are cross-compiling")
+  return ()
+endif ()
+
 if (NOT TARGET llvm_gtest)
-  build_gtest()
+  message(WARNING "Flang-RT unittests disabled due to GTest being unavailable; "
+                  "Try LLVM_INSTALL_GTEST=ON for the LLVM build")
+  return ()
 endif ()
 
+
+add_dependencies(flang-rt-test-depends
+  FlangRTUnitTests
+  flang_rt.runtime.unittest
+)
+
 if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
   add_compile_options("-Wno-suggest-override")
 endif()
 
 
-# Target that depends on all unittests
-add_custom_target(FlangRTUnitTests)
-set_target_properties(FlangRTUnitTests PROPERTIES FOLDER "Flang-RT/Meta")
-
-
 function(add_flangrt_unittest_offload_properties target)
   # Set CUDA_RESOLVE_DEVICE_SYMBOLS.
   if (FLANG_RT_EXPERIMENTAL_OFFLOAD_SUPPORT STREQUAL "CUDA")
diff --git a/offload/CMakeLists.txt b/offload/CMakeLists.txt
index 0a441c3bc5782..d49069f6eb420 100644
--- a/offload/CMakeLists.txt
+++ b/offload/CMakeLists.txt
@@ -380,15 +380,5 @@ add_subdirectory(liboffload)
 # Add tests.
 if(OFFLOAD_INCLUDE_TESTS)
   add_subdirectory(test)
-
-  # Add unit tests if GMock/GTest is present
-  if(NOT LLVM_THIRD_PARTY_DIR)
-    set(LLVM_THIRD_PARTY_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../third-party")
-  endif()
-  if(EXISTS ${LLVM_THIRD_PARTY_DIR}/unittest AND NOT TARGET llvm_gtest)
-    add_subdirectory(${LLVM_THIRD_PARTY_DIR}/unittest ${CMAKE_CURRENT_BINARY_DIR}/third-party/unittest)
-  endif()
-  if(TARGET llvm_gtest)
-    add_subdirectory(unittests)
-  endif()
+  add_subdirectory(unittests)
 endif()
diff --git a/offload/unittests/CMakeLists.txt b/offload/unittests/CMakeLists.txt
index 985dd892d8046..24ce0ec56e2d5 100644
--- a/offload/unittests/CMakeLists.txt
+++ b/offload/unittests/CMakeLists.txt
@@ -1,6 +1,20 @@
 add_custom_target(OffloadUnitTests)
 set_target_properties(OffloadUnitTests PROPERTIES FOLDER "Tests/UnitTests")
 
+if (CMAKE_CROSSCOMPILING)
+  # TODO: It is possible that LLVM_GTEST_RUN_UNDER defines an emulator or
+  #       ssh remote command invocation; for this case provide an option to
+  #       enable unittests.
+  message(STATUS "Offload unittests disabled because we are cross-compiling")
+  return ()
+endif ()
+
+if (NOT TARGET llvm_gtest)
+  message(WARNING "Offload unittests disabled due to GTest being unavailable; "
+                  "Try LLVM_INSTALL_GTEST=ON for the LLVM build")
+  return ()
+endif ()
+
 function(add_offload_unittest test_dirname)
   set(target_name "${test_dirname}.unittests")
 
diff --git a/third-party/unittest/CMakeLists.txt b/third-party/unittest/CMakeLists.txt
index 8b38791629a4e..ea3a310cbbfae 100644
--- a/third-party/unittest/CMakeLists.txt
+++ b/third-party/unittest/CMakeLists.txt
@@ -38,13 +38,13 @@ if (HAVE_LIBPTHREAD)
   list(APPEND LIBS pthread)
 endif()
 
-# Do not build unittest libraries automatically, they will be pulled in
-# by unittests if these are built.
+# Make available for runtimes using the LLVM buildtree
+# (required for unittests in bootstrapping builds)
+set(EXCLUDE_FROM_ALL OFF)
 
+# Install GTest only if requested.
 set(BUILDTREE_ONLY BUILDTREE_ONLY)
-set(EXCLUDE_FROM_ALL ON)
 if (LLVM_INSTALL_GTEST)
-  set(EXCLUDE_FROM_ALL OFF)
   set(BUILDTREE_ONLY "")
 endif ()
 
@@ -82,6 +82,16 @@ target_include_directories(llvm_gtest
   PRIVATE googletest googlemock
   )
 
+# When used from the buildtree, also force use of buildtree LLVM headers,
+# (instead locally installed version)
+# FIXME: Shouldn't this be done for all LLVM libraries? Currently, LLVM uses a
+# big giant `include_directories( ${LLVM_INCLUDE_DIR} ${LLVM_MAIN_INCLUDE_DIR})`
+# which CMake does not add to the import library.
+target_include_directories(llvm_gtest BEFORE
+  PUBLIC $<BUILD_INTERFACE:${LLVM_SOURCE_DIR}/include>
+         $<BUILD_INTERFACE:${LLVM_BINARY_DIR}/include>
+  )
+
 add_subdirectory(UnitTestMain)
 
 if (LLVM_INSTALL_GTEST)

[DavidSpickett]

ping @jhuber6