-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
[CMAKE][AMDGPU] fix build failure caused by PR #133619 #133776
Conversation
While clangCodeGenTargetBuiltins and clangCodeGenTargets are static libraries clangCodeGen is not and so this is creating a circular reference in the linux amdgpu-offload CIs on ubuntu and RHEL. removing the circular reference doesn't break anything and looks to be a vestigial element of the origional PR.
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-clang Author: Farzon Lotfi (farzonl) ChangesWhile clangCodeGenTargetBuiltins and clangCodeGenTargets are static libraries clangCodeGen is not and so this is creating a circular reference in the linux amdgpu-offload CIs on ubuntu and RHEL. removing the circular reference doesn't break anything and looks to be a vestigial element of the origional PR. Full diff: https://github.com/llvm/llvm-project/pull/133776.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt b/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt
index 8526c063b4593..76be68a11d02a 100644
--- a/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt
+++ b/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt
@@ -12,8 +12,3 @@ add_clang_library(clangCodeGenTargetBuiltins STATIC
WebAssembly.cpp
X86.cpp
)
-
-target_link_libraries(clangCodeGenTargetBuiltins
- PRIVATE
- clangCodeGen
-)
diff --git a/clang/lib/CodeGen/Targets/CMakeLists.txt b/clang/lib/CodeGen/Targets/CMakeLists.txt
index fd79b6191b379..6b6e7ce5b0b29 100644
--- a/clang/lib/CodeGen/Targets/CMakeLists.txt
+++ b/clang/lib/CodeGen/Targets/CMakeLists.txt
@@ -28,8 +28,3 @@ add_clang_library(clangCodeGenTargets STATIC
X86.cpp
XCore.cpp
)
-
-target_link_libraries(clangCodeGenTargets
- PRIVATE
- clangCodeGen
-)
|
@llvm/pr-subscribers-clang-codegen Author: Farzon Lotfi (farzonl) ChangesWhile clangCodeGenTargetBuiltins and clangCodeGenTargets are static libraries clangCodeGen is not and so this is creating a circular reference in the linux amdgpu-offload CIs on ubuntu and RHEL. removing the circular reference doesn't break anything and looks to be a vestigial element of the origional PR. Full diff: https://github.com/llvm/llvm-project/pull/133776.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt b/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt
index 8526c063b4593..76be68a11d02a 100644
--- a/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt
+++ b/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt
@@ -12,8 +12,3 @@ add_clang_library(clangCodeGenTargetBuiltins STATIC
WebAssembly.cpp
X86.cpp
)
-
-target_link_libraries(clangCodeGenTargetBuiltins
- PRIVATE
- clangCodeGen
-)
diff --git a/clang/lib/CodeGen/Targets/CMakeLists.txt b/clang/lib/CodeGen/Targets/CMakeLists.txt
index fd79b6191b379..6b6e7ce5b0b29 100644
--- a/clang/lib/CodeGen/Targets/CMakeLists.txt
+++ b/clang/lib/CodeGen/Targets/CMakeLists.txt
@@ -28,8 +28,3 @@ add_clang_library(clangCodeGenTargets STATIC
X86.cpp
XCore.cpp
)
-
-target_link_libraries(clangCodeGenTargets
- PRIVATE
- clangCodeGen
-)
|
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.
LGTM
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.
I tried to build it locally and I got a series of 'undefined reference' errors. Sorry, I will need to revert my approval.
Attn. @jhuber6
Thanks
@farzonl can you please revert the original commit? It seems to break just any LLVM build, so I guess the CI testing of all new PRs should be affected. |
I don't see it breaking any build in LLVM, just the amd offload ones. Al the Premerge tests also passed. I suspect the but the PR to revert is here #133795 |
Could you provide the failures you were seeing, specific build instructions, and platform you were doing this on. |
Thanks! I think, it only affects the A couple more examples: https://lab.llvm.org/buildbot/#/builders/145/builds/6156 https://lab.llvm.org/buildbot/#/builders/89/builds/19738 |
Thanks @vzakhari, I also am building with -DBUILD_SHARED_LIBS=ON |
While clangCodeGenTargetBuiltins and clangCodeGenTargets are static libraries clangCodeGen is not and so this is creating a circular reference in the linux amdgpu-offload CIs on ubuntu and RHEL.
removing the circular reference doesn't break anything and looks to be a vestigial element of the origional PR.