Skip to content

[llvm] update a few llvm unit tests to link statically #145448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

andrurogerz
Copy link
Contributor

@andrurogerz andrurogerz commented Jun 24, 2025

Purpose

Apply a few CMake changes so that LLVM unit tests can still build/run when LLVM is built as a Windows DLL.

Background

The effort to build LLVM as a WIndows DLL is tracked in #109483. Additional context is provided in this discourse.

Some LLVM unit tests use internal/private symbols that are not part of LLVM's public interface. When building LLVM as a DLL or shared library with default hidden symbol visibility, the symbols are not available when the unit test links against the DLL or shared library.

This problem can be solved in one of two ways:

  1. Export the private symbols from the DLL.
  2. Link the unit tests against the intermediate static libraries instead of the final LLVM DLL.

This PR applies option 2 because there is already precedence for it in the codebase. This solution is arguably more correct than exporting internal interfaces as public. The only downside is that the unit tests do not validate correctness of the LLVM DLL, but this is arguably not the purpose of a unit test anyway.

Overview

This PR makes the following changes:

  • Adds a new test library LLVMTestingSupportStatic which is identical to the existing LLVMTestingSupport library but it statically links its LLVM dependencies by passing the DISABLE_LLVM_LINK_LLVM_DYLIB argument to add_llvm_library.
  • Updates CodeGenTests, DebugInfoDWARFTests, DebugInfoLogicalViewTests, and FileCheckTests to link against the new LLVMTestingSupportStatic library and to link their LLVM dependencies statically using DISABLE_LLVM_LINK_LLVM_DYLIB.
  • Updates TableGenTests and VectorizeTests to link their LLVM dependencies statically using DISABLE_LLVM_LINK_LLVM_DYLIB.

@andrurogerz andrurogerz force-pushed the llvm-static-unittest branch from cbbce01 to bce8c8e Compare June 24, 2025 14:42
@andrurogerz andrurogerz changed the title Llvm static unittest [llvm] update a few llvm unit tests to link statically Jun 24, 2025
@andrurogerz andrurogerz force-pushed the llvm-static-unittest branch from bce8c8e to ccb5288 Compare June 24, 2025 15:56
@andrurogerz andrurogerz marked this pull request as ready for review June 24, 2025 16:56
@llvmbot
Copy link
Member

llvmbot commented Jun 24, 2025

@llvm/pr-subscribers-llvm-support

@llvm/pr-subscribers-testing-tools

Author: Andrew Rogers (andrurogerz)

Changes

Purpose

Apply a few CMake changes so that LLVM unit tests can still build/run when LLVM is built as a Windows DLL.

Background

The effort to build LLVM as a WIndows DLL is tracked in #109483. Additional context is provided in this discourse.

Some LLVM unit tests use internal/private symbols that are not part of LLVM's public interface. When building LLVM as a DLL or shared library with default hidden symbol visibility, the symbols are not available when the unit test links against the DLL or shared library.

This problem can be solved in one of two ways:

  1. Export the private symbols from the DLL.
  2. Link the unit tests against the intermediate static libraries instead of the final LLVM DLL.

This PR applies option 2 because there is already precedence for it in the codebase. This solution is arguably more correct than exporting internal interfaces as public. The only downside is that the unit tests do not validate correctness of the LLVM DLL, but this is arguably not the purpose of a unit test anyway.

Overview

This PR makes the following changes:

  • Adds a new test library LLVMTestingSupportStatic which is identical to the existing LLVMTestingSupport library but it statically links its LLVM dependencies by passing the DISABLE_LLVM_LINK_LLVM_DYLIB argument to add_llvm_library.
  • Updates CodeGenTests, DebugInfoDWARFTests, DebugInfoLogicalViewTests, and FileCheckTests to link against the new LLVMTestingSupportStatic library and to link their LLVM dependencies statically using DISABLE_LLVM_LINK_LLVM_DYLIB.
  • Updates TableGenTests and VectorizeTests to link their LLVM dependencies statically using DISABLE_LLVM_LINK_LLVM_DYLIB.

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

7 Files Affected:

  • (modified) llvm/lib/Testing/Support/CMakeLists.txt (+17)
  • (modified) llvm/unittests/CodeGen/CMakeLists.txt (+3-1)
  • (modified) llvm/unittests/DebugInfo/DWARF/CMakeLists.txt (+3-1)
  • (modified) llvm/unittests/DebugInfo/LogicalView/CMakeLists.txt (+3-1)
  • (modified) llvm/unittests/FileCheck/CMakeLists.txt (+3-1)
  • (modified) llvm/unittests/TableGen/CMakeLists.txt (+2)
  • (modified) llvm/unittests/Transforms/Vectorize/CMakeLists.txt (+2)
diff --git a/llvm/lib/Testing/Support/CMakeLists.txt b/llvm/lib/Testing/Support/CMakeLists.txt
index 6955271239ca6..425427b433f14 100644
--- a/llvm/lib/Testing/Support/CMakeLists.txt
+++ b/llvm/lib/Testing/Support/CMakeLists.txt
@@ -20,6 +20,23 @@ add_llvm_library(LLVMTestingSupport
 
 target_link_libraries(LLVMTestingSupport PRIVATE llvm_gtest)
 
+add_llvm_library(LLVMTestingSupportStatic
+  Error.cpp
+  SupportHelpers.cpp
+
+  ${BUILDTREE_ONLY}
+
+  ADDITIONAL_HEADER_DIRS
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Testing/Support
+
+  LINK_COMPONENTS
+  Support
+
+  DISABLE_LLVM_LINK_LLVM_DYLIB
+  )
+
+target_link_libraries(LLVMTestingSupportStatic PRIVATE llvm_gtest)
+
 # This is to avoid the error in gtest-death-test-internal.h
 # (150,16): error: 'Create' overrides a member function but
 # is not marked 'override' [-Werror,-Wsuggest-override]
diff --git a/llvm/unittests/CodeGen/CMakeLists.txt b/llvm/unittests/CodeGen/CMakeLists.txt
index 8b025219c46cf..9d3b8fc837a61 100644
--- a/llvm/unittests/CodeGen/CMakeLists.txt
+++ b/llvm/unittests/CodeGen/CMakeLists.txt
@@ -48,8 +48,10 @@ add_llvm_unittest(CodeGenTests
   TestAsmPrinter.cpp
   MLRegAllocDevelopmentFeatures.cpp
   X86MCInstLowerTest.cpp
+
+  DISABLE_LLVM_LINK_LLVM_DYLIB
   )
 
 add_subdirectory(GlobalISel)
 
-target_link_libraries(CodeGenTests PRIVATE LLVMTestingSupport)
+target_link_libraries(CodeGenTests PRIVATE LLVMTestingSupportStatic)
diff --git a/llvm/unittests/DebugInfo/DWARF/CMakeLists.txt b/llvm/unittests/DebugInfo/DWARF/CMakeLists.txt
index 0c5b3f28ca3d5..a8b3c7cf6a012 100644
--- a/llvm/unittests/DebugInfo/DWARF/CMakeLists.txt
+++ b/llvm/unittests/DebugInfo/DWARF/CMakeLists.txt
@@ -28,6 +28,8 @@ add_llvm_unittest(DebugInfoDWARFTests
   DWARFFormValueTest.cpp
   DWARFListTableTest.cpp
   DWARFLocationExpressionTest.cpp
+
+  DISABLE_LLVM_LINK_LLVM_DYLIB
   )
 
-target_link_libraries(DebugInfoDWARFTests PRIVATE LLVMTestingSupport)
+target_link_libraries(DebugInfoDWARFTests PRIVATE LLVMTestingSupportStatic)
diff --git a/llvm/unittests/DebugInfo/LogicalView/CMakeLists.txt b/llvm/unittests/DebugInfo/LogicalView/CMakeLists.txt
index 42a4b72229483..0d7d8c1d52254 100644
--- a/llvm/unittests/DebugInfo/LogicalView/CMakeLists.txt
+++ b/llvm/unittests/DebugInfo/LogicalView/CMakeLists.txt
@@ -22,6 +22,8 @@ add_llvm_unittest_with_input_files(DebugInfoLogicalViewTests
   LogicalElementsTest.cpp
   StringPoolTest.cpp
   WarningInternalTest.cpp
+
+  DISABLE_LLVM_LINK_LLVM_DYLIB
   )
 
-target_link_libraries(DebugInfoLogicalViewTests PRIVATE LLVMTestingSupport)
+target_link_libraries(DebugInfoLogicalViewTests PRIVATE LLVMTestingSupportStatic)
diff --git a/llvm/unittests/FileCheck/CMakeLists.txt b/llvm/unittests/FileCheck/CMakeLists.txt
index 7fe4f0c009d02..10de51cbe1d43 100644
--- a/llvm/unittests/FileCheck/CMakeLists.txt
+++ b/llvm/unittests/FileCheck/CMakeLists.txt
@@ -5,6 +5,8 @@ set(LLVM_LINK_COMPONENTS
 
 add_llvm_unittest(FileCheckTests
   FileCheckTest.cpp
+
+  DISABLE_LLVM_LINK_LLVM_DYLIB
 )
 
-target_link_libraries(FileCheckTests PRIVATE LLVMTestingSupport)
+target_link_libraries(FileCheckTests PRIVATE LLVMTestingSupportStatic)
diff --git a/llvm/unittests/TableGen/CMakeLists.txt b/llvm/unittests/TableGen/CMakeLists.txt
index 57b237306b19c..854f6c0f9b162 100644
--- a/llvm/unittests/TableGen/CMakeLists.txt
+++ b/llvm/unittests/TableGen/CMakeLists.txt
@@ -13,6 +13,8 @@ add_llvm_unittest(TableGenTests
   AutomataTest.cpp
   CodeExpanderTest.cpp
   ParserEntryPointTest.cpp
+
+  DISABLE_LLVM_LINK_LLVM_DYLIB
   )
 
 target_link_libraries(TableGenTests PRIVATE LLVMTableGenCommon LLVMTableGen)
diff --git a/llvm/unittests/Transforms/Vectorize/CMakeLists.txt b/llvm/unittests/Transforms/Vectorize/CMakeLists.txt
index 53eeff28c185f..4bed42e46cc2d 100644
--- a/llvm/unittests/Transforms/Vectorize/CMakeLists.txt
+++ b/llvm/unittests/Transforms/Vectorize/CMakeLists.txt
@@ -15,4 +15,6 @@ add_llvm_unittest(VectorizeTests
   VPlanPatternMatchTest.cpp
   VPlanSlpTest.cpp
   VPlanVerifierTest.cpp
+
+  DISABLE_LLVM_LINK_LLVM_DYLIB
   )

@andrurogerz
Copy link
Contributor Author

@compnerd, @vgvassilev this patch works-around a few LLVM unit tests that use internal interfaces by linking them statically with the LLVM component libraries they depend on instead of against the LLVM DLL. There are already a number of unit tests that were already building this way. The alternative solution is to annotate some headers under llvm/lib with LLVM_ABI even though they're not part of the public ABI. I think the approach in this PR is preferable, but I would appreciate your feedback. Thanks!

@nikic
Copy link
Contributor

nikic commented Jun 24, 2025

I'd like to see the alternative PR with ABI annotations to get an idea for the scope of the issue. Are we just talking about a handful of symbols here, or hundreds of them? Some of these tests (like CodeGen) depends on essentially ~everything, so forcing them to link statically is not ideal.

@andrurogerz
Copy link
Contributor Author

I'd like to see the alternative PR with ABI annotations to get an idea for the scope of the issue. Are we just talking about a handful of symbols here, or hundreds of them?

Sure, I will put together the alternative PR as well.

@andrurogerz
Copy link
Contributor Author

@nikic #145767 is the alternate approach of exporting private symbols to keep these unit tests building against the shared library. The individual commits in that PR illustrate what symbols are required for each of the tests.

NOTE: I believe it is correct for TableGenTests to link against the static libs as shown in this current PR because it links against LLVMTableGenCommon which also uses DISABLE_LLVM_LINK_LLVM_DYLIB. I changed this unit test to specify DISABLE_LLVM_LINK_LLVM_DYLIB to avoid duplicate symbols.

@nikic
Copy link
Contributor

nikic commented Jun 25, 2025

Thanks. I don't have a strong opinion here. The main downside to forcing static linking here is that we spend more time linking and take up more space. cc @rnk

By the way, I think for FileCheck, we should exclude that components from the dylib entirely -- it's only used by FileCheck, and utilities are always statically linked. So static linking for that test makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants