[libc] Allow hermetic timing if the clock function is built

[jhuber6]

Summary:
This patch fixes some code duplication on the GPU. The GPU build wanted
to enable timing for hermetic tests so it built some special case
handling into the test suite. Now that clock is supported on the
target we can simply link against the external interface. Because we
include clock.h for the CLOCKS_PER_SEC macro we remap the C entrypoint
to the internal one if it ends up called. This should allow hermetic
tests to run with timing if it is supported.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Joseph Huber (jhuber6)

Changes

Summary:
This patch fixes some code duplication on the GPU. The GPU build wanted
to enable timing for hermetic tests so it built some special case
handling into the test suite. Now that clock is supported on the
target we can simply link against the external interface. Because we
include clock.h for the CLOCKS_PER_SEC macro we remap the C entrypoint
to the internal one if it ends up called. This should allow hermetic
tests to run with timing if it is supported.

---

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

3 Files Affected:

• (modified) libc/cmake/modules/LLVMLibCTestRules.cmake (+6)
• (modified) libc/test/UnitTest/CMakeLists.txt (+3)
• (modified) libc/test/UnitTest/LibcTest.cpp (+7-15)

diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index fc66c129609456e..f7ca6a2e7021141 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -636,6 +636,12 @@ function(add_libc_hermetic_test test_name)
       libc.src.string.memset
       libc.src.__support.StringUtil.error_to_string
   )
+
+  if(TARGET libc.src.time.clock)
+    # We will link in the 'clock' implementation if it exists for test timing.
+    list(APPEND fq_deps_list libc.src.time.clock)
+  endif()
+
   list(REMOVE_DUPLICATES fq_deps_list)
 
   # TODO: Instead of gathering internal object files from entrypoints,
diff --git a/libc/test/UnitTest/CMakeLists.txt b/libc/test/UnitTest/CMakeLists.txt
index 4d384535581267d..9baa41874a83dba 100644
--- a/libc/test/UnitTest/CMakeLists.txt
+++ b/libc/test/UnitTest/CMakeLists.txt
@@ -28,6 +28,9 @@ function(add_unittest_framework_library name)
     )
     target_include_directories(${lib} PUBLIC ${LIBC_SOURCE_DIR})
     target_compile_options(${lib} PRIVATE -fno-exceptions -fno-rtti)
+    if(TARGET libc.src.time.clock)
+      target_compile_definitions(${lib} PRIVATE TARGET_SUPPORTS_CLOCK)
+    endif()
   endforeach()
   target_include_directories(${name}.hermetic PRIVATE ${LIBC_BUILD_DIR}/include)
   target_compile_options(${name}.hermetic
diff --git a/libc/test/UnitTest/LibcTest.cpp b/libc/test/UnitTest/LibcTest.cpp
index 5c27e2e69bead95..3fba95f88f0fb3e 100644
--- a/libc/test/UnitTest/LibcTest.cpp
+++ b/libc/test/UnitTest/LibcTest.cpp
@@ -15,20 +15,12 @@
 
 #if __STDC_HOSTED__
 #include <time.h>
-#elif defined(LIBC_TARGET_ARCH_IS_GPU)
-#include "src/__support/GPU/utils.h"
-static long clock() { return LIBC_NAMESPACE::gpu::fixed_frequency_clock(); }
-#if defined(LIBC_TARGET_ARCH_IS_NVPTX)
-#define CLOCKS_PER_SEC 1000000000UL
-#else
-// The AMDGPU loader needs to initialize this at runtime by querying the driver.
-extern "C" [[gnu::visibility("protected")]] uint64_t
-    [[clang::address_space(4)]] __llvm_libc_clock_freq;
-#define CLOCKS_PER_SEC __llvm_libc_clock_freq
-#endif
-#else
-static long clock() { return 0; }
-#define CLOCKS_PER_SEC 1
+#define LIBC_TEST_USE_CLOCK
+#elif defined(TARGET_SUPPORTS_CLOCK)
+#include <time.h>
+#include "src/time/clock.h"
+extern "C" clock_t clock() noexcept { return LIBC_NAMESPACE::clock(); }
+#define LIBC_TEST_USE_CLOCK
 #endif
 
 namespace LIBC_NAMESPACE {
@@ -147,7 +139,7 @@ int Test::runTests(const char *TestFilter) {
       break;
     case RunContext::RunResult::Pass:
       tlog << GREEN << "[       OK ] " << RESET << TestName;
-#if __STDC_HOSTED__ || defined(LIBC_TARGET_ARCH_IS_GPU)
+#ifdef LIBC_TEST_USE_CLOCK
       tlog << " (took ";
       if (start_time > end_time) {
         tlog << "unknown - try rerunning)\n";

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[michaelrj-google]

LGTM, with a place for potential future work

[michaelrj-google]

This change looks fine to me, though in future I think it would be useful to have it report the time regardless of if the test passes or fails.

[jhuber6]

This guards usage of the clock() function. We could potentially make it print something if unimplemented, or default to just return zero or something.