[Offload] Fix the offload cache file triggering libc++ / libstdc++ mixing #126313
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…xing Summary: We originally wanted `-stdlib=libc++` by default so that it could use offloading support in libc++, however this causes issues with out the Offloading proejct itself is built. Is the user builds the LLVM libs with libstdc++ then uses this cache it will enable this option by default for the ensuing build of the offloading libraries with the newly build clang. This will cause a lot of linker failured because the C++ library doesn't match. Long term I think the proper solution to this is to make better use of clang configuration files, but I don't know a good way to do that by default. For now just make it build right.
|
@llvm/pr-subscribers-offload Author: Joseph Huber (jhuber6) ChangesSummary: Long term I think the proper solution to this is to make better use of Full diff: https://github.com/llvm/llvm-project/pull/126313.diff 1 Files Affected:
diff --git a/offload/cmake/caches/Offload.cmake b/offload/cmake/caches/Offload.cmake
index 8749fcccb0ed1e8..5533a6508f5d57c 100644
--- a/offload/cmake/caches/Offload.cmake
+++ b/offload/cmake/caches/Offload.cmake
@@ -2,9 +2,6 @@ set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld" CACHE STRING "")
set(LLVM_ENABLE_RUNTIMES "compiler-rt;libunwind;libcxx;libcxxabi;openmp;offload" CACHE STRING "")
set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON CACHE BOOL "")
-set(CLANG_DEFAULT_CXX_STDLIB "libc++" CACHE STRING "")
-set(CLANG_DEFAULT_LINKER "lld" CACHE STRING "")
-
set(LLVM_RUNTIME_TARGETS default;amdgcn-amd-amdhsa;nvptx64-nvidia-cuda CACHE STRING "")
set(RUNTIMES_nvptx64-nvidia-cuda_CACHE_FILES "${CMAKE_SOURCE_DIR}/../libcxx/cmake/caches/NVPTX.cmake" CACHE STRING "")
set(RUNTIMES_amdgcn-amd-amdhsa_CACHE_FILES "${CMAKE_SOURCE_DIR}/../libcxx/cmake/caches/AMDGPU.cmake" CACHE STRING "")
|
jplehr
reviewed
Feb 10, 2025
| @@ -2,9 +2,6 @@ set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld" CACHE STRING "") | |||
| set(LLVM_ENABLE_RUNTIMES "compiler-rt;libunwind;libcxx;libcxxabi;openmp;offload" CACHE STRING "") | |||
| set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON CACHE BOOL "") | |||
|
|
|||
| set(CLANG_DEFAULT_CXX_STDLIB "libc++" CACHE STRING "") | |||
| set(CLANG_DEFAULT_LINKER "lld" CACHE STRING "") | |||
There was a problem hiding this comment.
Why also disable lld here as the default linker?
There was a problem hiding this comment.
It's not required for anything, probably shouldn't just default to it.
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…xing (llvm#126313) Summary: We originally wanted `-stdlib=libc++` by default so that it could use offloading support in libc++, however this causes issues with out the Offloading proejct itself is built. Is the user builds the LLVM libs with libstdc++ then uses this cache it will enable this option by default for the ensuing build of the offloading libraries with the newly build clang. This will cause a lot of linker failured because the C++ library doesn't match. Long term I think the proper solution to this is to make better use of clang configuration files, but I don't know a good way to do that by default. For now just make it build right.
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Feb 14, 2025
…xing (llvm#126313) Summary: We originally wanted `-stdlib=libc++` by default so that it could use offloading support in libc++, however this causes issues with out the Offloading proejct itself is built. Is the user builds the LLVM libs with libstdc++ then uses this cache it will enable this option by default for the ensuing build of the offloading libraries with the newly build clang. This will cause a lot of linker failured because the C++ library doesn't match. Long term I think the proper solution to this is to make better use of clang configuration files, but I don't know a good way to do that by default. For now just make it build right.
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Feb 24, 2025
…xing (llvm#126313) Summary: We originally wanted `-stdlib=libc++` by default so that it could use offloading support in libc++, however this causes issues with out the Offloading proejct itself is built. Is the user builds the LLVM libs with libstdc++ then uses this cache it will enable this option by default for the ensuing build of the offloading libraries with the newly build clang. This will cause a lot of linker failured because the C++ library doesn't match. Long term I think the proper solution to this is to make better use of clang configuration files, but I don't know a good way to do that by default. For now just make it build right.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
We originally wanted
-stdlib=libc++by default so that it could useoffloading support in libc++, however this causes issues with out the
Offloading proejct itself is built. Is the user builds the LLVM libs
with libstdc++ then uses this cache it will enable this option by
default for the ensuing build of the offloading libraries with the newly
build clang. This will cause a lot of linker failured because the C++
library doesn't match.
Long term I think the proper solution to this is to make better use of
clang configuration files, but I don't know a good way to do that by
default. For now just make it build right.