-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[BOLT] Fix unit test failures with LLVM_LINK_LLVM_DYLIB=ON #152190
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
Conversation
When LLVM_LINK_LLVM_DYLIB is ON, `check-bolt` target reports unit test failures: BOLT-Unit :: Core/./CoreTests/failed_to_discover_tests_from_gtest BOLT-Unit :: Profile/./ProfileTests/failed_to_discover_tests_from_gtest The reason is that when llvm-lit runs a unit-test executable: /path/to/CoreTests --gtest_list_tests '--gtest_filter=-*DISABLED_*' an assertion is triggered with the following message: LLVM ERROR: Option 'default' already exists! This assertion triggers when the initializer of defaultListDAGScheduler defined at SelectionDAGISel.cpp:219 is called as a statically-linked function after already being called during the initialization of libLLVM. The issue can be traced down to LLVMTestingSupport library which depends on libLLVM as neither COMPONENT_LIB nor DISABLE_LLVM_LINK_LLVM_DYLIB is specified in a call to `add_llvm_library(LLVMTestingSupport ...)`. Specifying DISABLE_LLVM_LINK_LLVM_DYLIB for LLVMTestingSupport makes Clang unit test fail and COMPONENT_LIB is probably inappropriate for a testing-specific library, thus as a workaround, added Error.cpp source from LLVMTestingSupport directly to the list of source files of CoreTests target (as it depends on `llvm::detail::TakeError(llvm::Error)`) and removed LLVMTestingSupport from the list of dependencies of ProfileTests.
@llvm/pr-subscribers-bolt Author: Anatoly Trosinenko (atrosinenko) ChangesWhen LLVM_LINK_LLVM_DYLIB is ON,
The reason is that when llvm-lit runs a unit-test executable:
an assertion is triggered with the following message:
This assertion triggers when the initializer of defaultListDAGScheduler defined at SelectionDAGISel.cpp:219 is called as a statically-linked function after already being called during the initialization of libLLVM. The issue can be traced down to LLVMTestingSupport library which depends on libLLVM as neither COMPONENT_LIB nor DISABLE_LLVM_LINK_LLVM_DYLIB is specified in a call to Specifying DISABLE_LLVM_LINK_LLVM_DYLIB for LLVMTestingSupport makes Clang unit test fail and COMPONENT_LIB is probably inappropriate for a testing-specific library, thus as a workaround, added Error.cpp source from LLVMTestingSupport directly to the list of source files of CoreTests target (as it depends on Full diff: https://github.com/llvm/llvm-project/pull/152190.diff 2 Files Affected:
diff --git a/bolt/unittests/Core/CMakeLists.txt b/bolt/unittests/Core/CMakeLists.txt
index 54e8ea10cda12..f10b0d9472067 100644
--- a/bolt/unittests/Core/CMakeLists.txt
+++ b/bolt/unittests/Core/CMakeLists.txt
@@ -11,6 +11,11 @@ add_bolt_unittest(CoreTests
MemoryMaps.cpp
DynoStats.cpp
+ # FIXME CoreTests uses `llvm::detail::TakeError(llvm::Error)`, but linking
+ # to LLVMTestingSupport introduces a transitive dependency on the
+ # dynamic LLVM library when LLVM_LINK_LLVM_DYLIB is ON.
+ ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support/Error.cpp
+
DISABLE_LLVM_LINK_LLVM_DYLIB
)
@@ -20,7 +25,6 @@ target_link_libraries(CoreTests
LLVMBOLTRewrite
LLVMBOLTProfile
LLVMBOLTUtils
- LLVMTestingSupport
)
foreach (tgt ${BOLT_TARGETS_TO_BUILD})
diff --git a/bolt/unittests/Profile/CMakeLists.txt b/bolt/unittests/Profile/CMakeLists.txt
index ce01c6c4b949e..7b3cbd2cad724 100644
--- a/bolt/unittests/Profile/CMakeLists.txt
+++ b/bolt/unittests/Profile/CMakeLists.txt
@@ -16,7 +16,6 @@ target_link_libraries(ProfileTests
LLVMBOLTCore
LLVMBOLTProfile
LLVMTargetParser
- LLVMTestingSupport
)
foreach (tgt ${BOLT_TARGETS_TO_BUILD})
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @atrosinenko,
Thanks for your patch. I reproduced this issue locally with both DYLIB
and ASSERTIONS
enabled just before your fix.
However, our bolt-aarch64-ubuntu-dylib
also compiles with DYLIB=ON
:
and the latest build #634 does not hit this issue.
Compiling locally again, with DYLIB=ON
and ASSERTIONS=OFF
does not fail, so ASSERTIONS=OFF
seems to be the cause.
I believe this matches your findings; can you confirm?
Yes, I tried rebuilding LLVM from scratch at cd02680 with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks @atrosinenko!
When LLVM_LINK_LLVM_DYLIB is ON,
check-bolt
target reports unit test failures:The reason is that when llvm-lit runs a unit-test executable:
an assertion is triggered with the following message:
This assertion triggers when the initializer of defaultListDAGScheduler defined at SelectionDAGISel.cpp:219 is called as a statically-linked function after already being called during the initialization of libLLVM.
The issue can be traced down to LLVMTestingSupport library which depends on libLLVM as neither COMPONENT_LIB nor DISABLE_LLVM_LINK_LLVM_DYLIB is specified in a call to
add_llvm_library(LLVMTestingSupport ...)
.Specifying DISABLE_LLVM_LINK_LLVM_DYLIB for LLVMTestingSupport makes Clang unit test fail and COMPONENT_LIB is probably inappropriate for a testing-specific library, thus as a workaround, added Error.cpp source from LLVMTestingSupport directly to the list of source files of CoreTests target (as it depends on
llvm::detail::TakeError(llvm::Error)
) and removed LLVMTestingSupport from the list of dependencies of ProfileTests.