[OpenMP][omptest] Improve CMake and address review comments

[mhalk]

Avoid explicit ABI breaking check deactivation
Replace whole-archive linking with dedicated build of GoogleTest lib

Addresses remaining post-merge review comments of #154786

[jplehr]

I'm not of much help here.

[DavidSpickett]

Our flang built with libcxx bot has been broken since #154786 landed, that's our fault for leaving it so long.

So I've just got to trying this in case it helped, and I get:

[9/11] Building CXX object openmp/tools/omptest/CMakeFiles/omptest.dir/src/OmptTester.cpp.o
FAILED: openmp/tools/omptest/CMakeFiles/omptest.dir/src/OmptTester.cpp.o 
/home/tcwg-buildbot/build-llvm-flang/./bin/clang++ --target=aarch64-unknown-linux-gnu -D_DEBUG -D_GLIBCXX_ASSERTIONS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Domptest_EXPORTS -I/home/tcwg-buildbot/build-llvm-flang/runtimes/runtimes-bins/openmp/runtime/src -I/home/tcwg-buildbot/llvm-project/llvm/include -I/home/tcwg-buildbot/build-llvm-flang/include -I/home/tcwg-buildbot/llvm-project/openmp/tools/omptest/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Wall -fcolor-diagnostics -Wcast-qual -Wformat-pedantic -Wimplicit-fallthrough -Wsign-compare -Wno-extra -Wno-pedantic -fno-semantic-interposition -fdata-sections -O3 -DNDEBUG -std=c++17 -fPIC -fvisibility=default -UNDEBUG -MD -MT openmp/tools/omptest/CMakeFiles/omptest.dir/src/OmptTester.cpp.o -MF openmp/tools/omptest/CMakeFiles/omptest.dir/src/OmptTester.cpp.o.d -o openmp/tools/omptest/CMakeFiles/omptest.dir/src/OmptTester.cpp.o -c /home/tcwg-buildbot/llvm-project/openmp/tools/omptest/src/OmptTester.cpp
In file included from /home/tcwg-buildbot/llvm-project/openmp/tools/omptest/src/OmptTester.cpp:17:
In file included from /home/tcwg-buildbot/llvm-project/openmp/tools/omptest/include/OmptTester.h:49:
/home/tcwg-buildbot/llvm-project/openmp/tools/omptest/include/OmptTesterGoogleTest.h:27:10: fatal error: 'gtest/gtest.h' file not found
   27 | #include "gtest/gtest.h"
      |          ^~~~~~~~~~~~~~~
1 error generated.
ninja: build stopped: subcommand failed.

I think this is happening because the include paths are added for omptest_gtest but not for omptest. This change "fixes" it:

diff --git a/openmp/tools/omptest/CMakeLists.txt b/openmp/tools/omptest/CMakeLists.txt
index e3af71c856ba..8e97d0dd00c5 100644
--- a/openmp/tools/omptest/CMakeLists.txt
+++ b/openmp/tools/omptest/CMakeLists.txt
@@ -84,6 +84,11 @@ if ((NOT LIBOMPTEST_BUILD_STANDALONE) OR LIBOMPTEST_BUILD_UNITTESTS)
     "${OMPTEST_GTEST_PATH}/include"
   )
 
+  target_include_directories(omptest PRIVATE
+    "${OMPTEST_GTEST_PATH}"
+    "${OMPTEST_GTEST_PATH}/include"
+  )
+
   # Avoid using LLVM-specific ostream helpers inside GoogleTest.
   target_compile_definitions(omptest_gtest PRIVATE GTEST_NO_LLVM_SUPPORT=1)

Does that make sense to you?

[DavidSpickett]

The cmake command I'm using:

cmake -DLLVM_TARGETS_TO_BUILD=AArch64 -DCMAKE_CXX_STANDARD=17 -DLLVM_ENABLE_WERROR=OFF -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_LIBCXX=ON -DCMAKE_BUILD_TYPE=Release '-DLLVM_ENABLE_PROJECTS=llvm;clang' '-DLLVM_ENABLE_RUNTIMES=openmp;compiler-rt' -DCMAKE_INSTALL_PREFIX=../install '-DLLVM_LIT_ARGS=-v -vv' -GNinja ../llvm-project/llvm -DLLVM_ENABLE_LLD=ON

Perhaps it's having it as a runtime that creates the problem but I expect that's the default now.

[mhalk]

@DavidSpickett Apologies for breaking your bot, honestly did not notice :/

Does that make sense to you?

Yes, I think that makes sense.
My build environment must have picked up the system GTest headers such that I did not run into issues.

Now, I have added the include directories to the omptest target, too.
If possible, please confirm if that fixes the issue.

Thank you for bringing this to my attention and taking the time to analyze.

[jplehr]

I just got back to omptest. I want to enable it in our buildbots (I thought I had already) in the CMake cache file we use via set(LIBOMPTEST_BUILD_UNITTESTS ON CACHE BOOL "").

Without this patch, I ran into liker errors when I tested it. With this patch, it works and I can execute the unit tests.

[DavidSpickett]

Apologies for breaking your bot, honestly did not notice :/

No worries, we would have done the investigation for you anyway because it's a rather involved build process.

My build environment must have picked up the system GTest headers such that I did not run into issues.

Oh yeah, this has happened pretty often so that explains it.

Going to test the build again today.

[DavidSpickett]

This PR gets our build past the first stage. I'm not sure about the rest because it's been broken so long I don't have examples of the commands.

So this may not fix all of our issues, but that's on me to figure out. If this works for @jplehr and the rest of the reviewers, then please go ahead with these changes and I will deal with whatever the result is on the bot.

[mhalk]

@nikic If you find the time to look at this, that would be very helpful :)
Also, you had the most input on the previous PR.