[clang] Merge gtest binaries into AllClangUnitTests #134196

rnk · 2025-04-03T04:13:48Z

This reduces the size of the clang/unittests build directory by 64% and my overall build dir size by 5%. Static linking is the real driving factor here, but even if the default build configuration used shared libraries, I don't see why we should be building so many unit test binaries.

To make the project more approachable for new contributors, I'm attempting to make the build a bit less resource-intensive. Build directory size is a common complaint, and this is low-hanging fruit.

This reduces the size of the clang/unittests build directory by 64% and my overall build dir size by 5%. Static linking is the real driving factor here, but even if the default build configuration used shared libraries, I don't see why we should be building so many unit test binaries. To make the project more approachable for new contributors, I'm attempting to make the build a bit less resource-intensive. Build directory size is a common complaint, and this is low-hanging fruit.

Copilot

Pull Request Overview

This PR renames the gtest binary for unit tests to reduce build directory size and overall resource usage.

Renames a test case from ModuleCacheTest to DriverModuleCacheTest
Aligns unit test naming with the goal of merging binaries

Files not reviewed (3)

clang/unittests/CMakeLists.txt: Language not supported
clang/unittests/Interpreter/CMakeLists.txt: Language not supported
clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt: Language not supported

Comments suppressed due to low confidence (1)

clang/unittests/Driver/ModuleCacheTest.cpp:20

[nitpick] The test case name 'DriverModuleCacheTest' is inconsistent with the file name 'ModuleCacheTest.cpp'. Consider renaming either the file or the test case for clarity.

TEST(DriverModuleCacheTest, GetTargetAndMode) {

llvmbot · 2025-04-03T04:14:19Z

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang-driver

Author: Reid Kleckner (rnk)

Changes

This reduces the size of the clang/unittests build directory by 64% and my overall build dir size by 5%. Static linking is the real driving factor here, but even if the default build configuration used shared libraries, I don't see why we should be building so many unit test binaries.

To make the project more approachable for new contributors, I'm attempting to make the build a bit less resource-intensive. Build directory size is a common complaint, and this is low-hanging fruit.

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

4 Files Affected:

(modified) clang/unittests/CMakeLists.txt (+51-2)
(modified) clang/unittests/Driver/ModuleCacheTest.cpp (+1-1)
(modified) clang/unittests/Interpreter/CMakeLists.txt (+1-1)
(modified) clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt (+1-1)

diff --git a/clang/unittests/CMakeLists.txt b/clang/unittests/CMakeLists.txt
index f3823ba309420..8d4476761e03e 100644
--- a/clang/unittests/CMakeLists.txt
+++ b/clang/unittests/CMakeLists.txt
@@ -15,11 +15,11 @@ if(CLANG_BUILT_STANDALONE)
   endif()
 endif()
 
-# add_clang_unittest(test_name file1.cpp file2.cpp)
+# add_distinct_clang_unittest(test_name file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against the clang
 # Produces a binary named 'basename(test_name)'.
-function(add_clang_unittest test_name)
+function(add_distinct_clang_unittest test_name)
   cmake_parse_arguments(ARG
     ""
     ""
@@ -47,6 +47,33 @@ function(add_clang_unittest test_name)
   target_link_libraries(${test_name} PRIVATE ${ARG_LINK_LIBS})
 endfunction()
 
+define_property(GLOBAL PROPERTY CLANG_UNITTEST_SRCS)
+define_property(GLOBAL PROPERTY CLANG_UNITTEST_LLVM_COMPONENTS)
+define_property(GLOBAL PROPERTY CLANG_UNITTEST_CLANG_LIBS)
+define_property(GLOBAL PROPERTY CLANG_UNITTEST_LINK_LIBS)
+
+# add_clang_unittest(test_name file1.cpp file2.cpp)
+#
+# Adds unittests to the combined AllClangUnitTests binary. The unittest binary
+# is defined after adding all unittest subdirectories.
+function(add_clang_unittest test_name)
+  cmake_parse_arguments(ARG
+    ""
+    ""
+    "CLANG_LIBS;LINK_LIBS;LLVM_COMPONENTS"
+    ${ARGN})
+
+  file(RELATIVE_PATH src_prefix "${CMAKE_CURRENT_FUNCTION_LIST_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}")
+  set(srcs_prefixed)
+  foreach(src ${ARG_UNPARSED_ARGUMENTS})
+    set(srcs_prefixed ${srcs_prefixed} "${src_prefix}/${src}")
+  endforeach()
+  set_property(GLOBAL APPEND PROPERTY CLANG_UNITTEST_SRCS ${srcs_prefixed})
+  set_property(GLOBAL APPEND PROPERTY CLANG_UNITTEST_CLANG_LIBS ${ARG_CLANG_LIBS})
+  set_property(GLOBAL APPEND PROPERTY CLANG_UNITTEST_LINK_LIBS ${ARG_LINK_LIBS})
+  set_property(GLOBAL APPEND PROPERTY CLANG_UNITTEST_LLVM_COMPONENTS ${ARG_LLVM_COMPONENTS})
+endfunction()
+
 add_subdirectory(Basic)
 add_subdirectory(Lex)
 add_subdirectory(Parse)
@@ -77,3 +104,25 @@ add_subdirectory(Index)
 add_subdirectory(InstallAPI)
 add_subdirectory(Serialization)
 add_subdirectory(Support)
+
+
+# If we're doing a single merged clang unit test binary, add that target after
+# all the previous subdirectories have been processed.
+get_property(SRCS GLOBAL PROPERTY CLANG_UNITTEST_SRCS)
+get_property(CLANG_LIBS GLOBAL PROPERTY CLANG_UNITTEST_CLANG_LIBS)
+get_property(LINK_LIBS GLOBAL PROPERTY CLANG_UNITTEST_LINK_LIBS)
+get_property(LLVM_COMPONENTS GLOBAL PROPERTY CLANG_UNITTEST_LLVM_COMPONENTS)
+add_distinct_clang_unittest(AllClangUnitTests
+  ${SRCS}
+  CLANG_LIBS
+  ${CLANG_LIBS}
+  LINK_LIBS
+  ${LINK_LIBS}
+  LLVM_COMPONENTS
+  ${LLVM_COMPONENTS}
+)
+
+# The Tooling library has some internal headers. Make those work. If we like
+# the merged clang unit test binary, we can udpate the include paths and make
+# this the default.
+include_directories(Tooling)
diff --git a/clang/unittests/Driver/ModuleCacheTest.cpp b/clang/unittests/Driver/ModuleCacheTest.cpp
index 48744415647e6..7aa5047c59bd3 100644
--- a/clang/unittests/Driver/ModuleCacheTest.cpp
+++ b/clang/unittests/Driver/ModuleCacheTest.cpp
@@ -17,7 +17,7 @@ using namespace clang::driver;
 
 namespace {
 
-TEST(ModuleCacheTest, GetTargetAndMode) {
+TEST(DriverModuleCacheTest, GetTargetAndMode) {
   SmallString<128> Buf;
   Driver::getDefaultModuleCachePath(Buf);
   StringRef Path = Buf;
diff --git a/clang/unittests/Interpreter/CMakeLists.txt b/clang/unittests/Interpreter/CMakeLists.txt
index 9df1a4b03da47..1dda9024075a1 100644
--- a/clang/unittests/Interpreter/CMakeLists.txt
+++ b/clang/unittests/Interpreter/CMakeLists.txt
@@ -1,4 +1,4 @@
-add_clang_unittest(ClangReplInterpreterTests
+add_distinct_clang_unittest(ClangReplInterpreterTests
   IncrementalCompilerBuilderTest.cpp
   IncrementalProcessingTest.cpp
   InterpreterTest.cpp
diff --git a/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt b/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt
index eb366a860661c..dfd94d8e6442c 100644
--- a/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt
+++ b/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt
@@ -3,7 +3,7 @@
 set(LLVM_REQUIRES_EH ON)
 set(LLVM_REQUIRES_RTTI ON)
 
-add_clang_unittest(ClangReplInterpreterExceptionTests
+add_distinct_clang_unittest(ClangReplInterpreterExceptionTests
   InterpreterExceptionTest.cpp
   EXPORT_SYMBOLS

llvmbot · 2025-04-03T04:14:20Z

@llvm/pr-subscribers-clang

Author: Reid Kleckner (rnk)

Changes

This reduces the size of the clang/unittests build directory by 64% and my overall build dir size by 5%. Static linking is the real driving factor here, but even if the default build configuration used shared libraries, I don't see why we should be building so many unit test binaries.

To make the project more approachable for new contributors, I'm attempting to make the build a bit less resource-intensive. Build directory size is a common complaint, and this is low-hanging fruit.

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

4 Files Affected:

(modified) clang/unittests/CMakeLists.txt (+51-2)
(modified) clang/unittests/Driver/ModuleCacheTest.cpp (+1-1)
(modified) clang/unittests/Interpreter/CMakeLists.txt (+1-1)
(modified) clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt (+1-1)

diff --git a/clang/unittests/CMakeLists.txt b/clang/unittests/CMakeLists.txt
index f3823ba309420..8d4476761e03e 100644
--- a/clang/unittests/CMakeLists.txt
+++ b/clang/unittests/CMakeLists.txt
@@ -15,11 +15,11 @@ if(CLANG_BUILT_STANDALONE)
   endif()
 endif()
 
-# add_clang_unittest(test_name file1.cpp file2.cpp)
+# add_distinct_clang_unittest(test_name file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against the clang
 # Produces a binary named 'basename(test_name)'.
-function(add_clang_unittest test_name)
+function(add_distinct_clang_unittest test_name)
   cmake_parse_arguments(ARG
     ""
     ""
@@ -47,6 +47,33 @@ function(add_clang_unittest test_name)
   target_link_libraries(${test_name} PRIVATE ${ARG_LINK_LIBS})
 endfunction()
 
+define_property(GLOBAL PROPERTY CLANG_UNITTEST_SRCS)
+define_property(GLOBAL PROPERTY CLANG_UNITTEST_LLVM_COMPONENTS)
+define_property(GLOBAL PROPERTY CLANG_UNITTEST_CLANG_LIBS)
+define_property(GLOBAL PROPERTY CLANG_UNITTEST_LINK_LIBS)
+
+# add_clang_unittest(test_name file1.cpp file2.cpp)
+#
+# Adds unittests to the combined AllClangUnitTests binary. The unittest binary
+# is defined after adding all unittest subdirectories.
+function(add_clang_unittest test_name)
+  cmake_parse_arguments(ARG
+    ""
+    ""
+    "CLANG_LIBS;LINK_LIBS;LLVM_COMPONENTS"
+    ${ARGN})
+
+  file(RELATIVE_PATH src_prefix "${CMAKE_CURRENT_FUNCTION_LIST_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}")
+  set(srcs_prefixed)
+  foreach(src ${ARG_UNPARSED_ARGUMENTS})
+    set(srcs_prefixed ${srcs_prefixed} "${src_prefix}/${src}")
+  endforeach()
+  set_property(GLOBAL APPEND PROPERTY CLANG_UNITTEST_SRCS ${srcs_prefixed})
+  set_property(GLOBAL APPEND PROPERTY CLANG_UNITTEST_CLANG_LIBS ${ARG_CLANG_LIBS})
+  set_property(GLOBAL APPEND PROPERTY CLANG_UNITTEST_LINK_LIBS ${ARG_LINK_LIBS})
+  set_property(GLOBAL APPEND PROPERTY CLANG_UNITTEST_LLVM_COMPONENTS ${ARG_LLVM_COMPONENTS})
+endfunction()
+
 add_subdirectory(Basic)
 add_subdirectory(Lex)
 add_subdirectory(Parse)
@@ -77,3 +104,25 @@ add_subdirectory(Index)
 add_subdirectory(InstallAPI)
 add_subdirectory(Serialization)
 add_subdirectory(Support)
+
+
+# If we're doing a single merged clang unit test binary, add that target after
+# all the previous subdirectories have been processed.
+get_property(SRCS GLOBAL PROPERTY CLANG_UNITTEST_SRCS)
+get_property(CLANG_LIBS GLOBAL PROPERTY CLANG_UNITTEST_CLANG_LIBS)
+get_property(LINK_LIBS GLOBAL PROPERTY CLANG_UNITTEST_LINK_LIBS)
+get_property(LLVM_COMPONENTS GLOBAL PROPERTY CLANG_UNITTEST_LLVM_COMPONENTS)
+add_distinct_clang_unittest(AllClangUnitTests
+  ${SRCS}
+  CLANG_LIBS
+  ${CLANG_LIBS}
+  LINK_LIBS
+  ${LINK_LIBS}
+  LLVM_COMPONENTS
+  ${LLVM_COMPONENTS}
+)
+
+# The Tooling library has some internal headers. Make those work. If we like
+# the merged clang unit test binary, we can udpate the include paths and make
+# this the default.
+include_directories(Tooling)
diff --git a/clang/unittests/Driver/ModuleCacheTest.cpp b/clang/unittests/Driver/ModuleCacheTest.cpp
index 48744415647e6..7aa5047c59bd3 100644
--- a/clang/unittests/Driver/ModuleCacheTest.cpp
+++ b/clang/unittests/Driver/ModuleCacheTest.cpp
@@ -17,7 +17,7 @@ using namespace clang::driver;
 
 namespace {
 
-TEST(ModuleCacheTest, GetTargetAndMode) {
+TEST(DriverModuleCacheTest, GetTargetAndMode) {
   SmallString<128> Buf;
   Driver::getDefaultModuleCachePath(Buf);
   StringRef Path = Buf;
diff --git a/clang/unittests/Interpreter/CMakeLists.txt b/clang/unittests/Interpreter/CMakeLists.txt
index 9df1a4b03da47..1dda9024075a1 100644
--- a/clang/unittests/Interpreter/CMakeLists.txt
+++ b/clang/unittests/Interpreter/CMakeLists.txt
@@ -1,4 +1,4 @@
-add_clang_unittest(ClangReplInterpreterTests
+add_distinct_clang_unittest(ClangReplInterpreterTests
   IncrementalCompilerBuilderTest.cpp
   IncrementalProcessingTest.cpp
   InterpreterTest.cpp
diff --git a/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt b/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt
index eb366a860661c..dfd94d8e6442c 100644
--- a/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt
+++ b/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt
@@ -3,7 +3,7 @@
 set(LLVM_REQUIRES_EH ON)
 set(LLVM_REQUIRES_RTTI ON)
 
-add_clang_unittest(ClangReplInterpreterExceptionTests
+add_distinct_clang_unittest(ClangReplInterpreterExceptionTests
   InterpreterExceptionTest.cpp
   EXPORT_SYMBOLS

clang/unittests/CMakeLists.txt

…odel

Co-authored-by: Petr Hosek <phosek@google.com>

…present

github-actions · 2025-04-10T14:42:54Z

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

rnk · 2025-04-11T07:38:22Z

This is ready now, so please take a look. Apologies for all the push notifications. It took a lot of retries to get this to pass the premerge tests due to cmake environmental differences, which I guess shows the value of shared premerge testing.

rnk · 2025-04-25T02:45:23Z

Ping, WDYT?

AaronBallman · 2025-04-25T13:56:57Z

clang/lib/CodeGen/BackendUtil.cpp

@@ -304,7 +304,7 @@ getCodeModel(const CodeGenOptions &CodeGenOpts) {
                           .Case("kernel", llvm::CodeModel::Kernel)
                           .Case("medium", llvm::CodeModel::Medium)
                           .Case("large", llvm::CodeModel::Large)
-                           .Case("default", ~1u)
+                           .Cases("default", "", ~1u)


Can you explain why this change is needed?

rnk requested a review from Copilot April 3, 2025 04:13

Copilot AI reviewed Apr 3, 2025

View reviewed changes

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Apr 3, 2025

rnk requested review from petrhosek, AaronBallman and aeubanks April 3, 2025 04:14

Migrate new ParseTests unittest binary to new style

4ca1b47

rnk marked this pull request as draft April 3, 2025 04:53

petrhosek reviewed Apr 3, 2025

View reviewed changes

clang/unittests/CMakeLists.txt Outdated Show resolved Hide resolved

rnk and others added 4 commits April 10, 2025 10:33

Merge branch 'main' into merge-clang-unittests

85582c4

Interpret an empty code model as no code model, or the default code m…

30a54f3

…odel

Update clang/unittests/CMakeLists.txt

3059cfa

Co-authored-by: Petr Hosek <phosek@google.com>

Initialize MCs so target machine construction works when targets are …

12069f8

…present

rnk marked this pull request as ready for review April 10, 2025 14:42

llvmbot added the clang:codegen IR generation bugs: mangling, exceptions, etc. label Apr 10, 2025

rnk added 3 commits April 10, 2025 16:43

Fix include lint

7c3e7a2

Satisfy non-optional brief doc requirement for premerge

db2fb78

Also set FULL_DOCS =S

eba6bce

AaronBallman reviewed Apr 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang] Merge gtest binaries into AllClangUnitTests #134196

[clang] Merge gtest binaries into AllClangUnitTests #134196

rnk commented Apr 3, 2025

Copilot AI left a comment

llvmbot commented Apr 3, 2025 •

edited

Loading

llvmbot commented Apr 3, 2025

github-actions bot commented Apr 10, 2025 •

edited

Loading

rnk commented Apr 11, 2025

rnk commented Apr 25, 2025

AaronBallman Apr 25, 2025

[clang] Merge gtest binaries into AllClangUnitTests #134196

Are you sure you want to change the base?

[clang] Merge gtest binaries into AllClangUnitTests #134196

Conversation

rnk commented Apr 3, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

llvmbot commented Apr 3, 2025 • edited Loading

llvmbot commented Apr 3, 2025

github-actions bot commented Apr 10, 2025 • edited Loading

rnk commented Apr 11, 2025

rnk commented Apr 25, 2025

AaronBallman Apr 25, 2025

Choose a reason for hiding this comment

llvmbot commented Apr 3, 2025 •

edited

Loading

github-actions bot commented Apr 10, 2025 •

edited

Loading