Skip to content
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

[offload] - Add omp as a dependency for clang-bootstrap-deps #90793

Merged
merged 1 commit into from
May 2, 2024

Conversation

estewart08
Copy link
Contributor

If openmp is on the LLVM_ENABLE_PROJECTS list and
offload is on LLVM_ENABLE_RUNTIMES list when using CLANG_ENABLE_BOOTSTRAP, then the runtimes will be
configured before the openmp project. This will throw a cannot find libomp.so dependency error. Add omp as a dependency when this is the case. Update
offload cmake for detection of LIBOMP_HAVE_VERSION_SCRIPT.

If openmp is on the LLVM_ENABLE_PROJECTS list and
offload is on LLVM_ENABLE_RUNTIMES list when using
CLANG_ENABLE_BOOTSTRAP, then the runtimes will be
configured before the openmp project. This will throw
a cannot find libomp.so dependency error. Add omp as
a dependency when this is the case. Update
offload cmake for detection of LIBOMP_HAVE_VERSION_SCRIPT.
@llvmbot
Copy link
Collaborator

llvmbot commented May 1, 2024

@llvm/pr-subscribers-offload

Author: None (estewart08)

Changes

If openmp is on the LLVM_ENABLE_PROJECTS list and
offload is on LLVM_ENABLE_RUNTIMES list when using CLANG_ENABLE_BOOTSTRAP, then the runtimes will be
configured before the openmp project. This will throw a cannot find libomp.so dependency error. Add omp as a dependency when this is the case. Update
offload cmake for detection of LIBOMP_HAVE_VERSION_SCRIPT.


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

2 Files Affected:

  • (modified) llvm/runtimes/CMakeLists.txt (+7)
  • (modified) offload/CMakeLists.txt (+8-5)
diff --git a/llvm/runtimes/CMakeLists.txt b/llvm/runtimes/CMakeLists.txt
index 8159d7f8a0a104..3020ba72f4a60b 100644
--- a/llvm/runtimes/CMakeLists.txt
+++ b/llvm/runtimes/CMakeLists.txt
@@ -559,6 +559,13 @@ if(runtimes)
     # We need to add the runtimes as a dependency because compiler-rt can be
     # built as part of runtimes and we need the profile runtime for PGO
     add_dependencies(clang-bootstrap-deps runtimes)
+    # The bootstrap build will attempt to configure the offload runtime
+    # before the openmp project which will error out due to failing to
+    # find libomp.so. We must add omp as a dependency before runtimes
+    # are configured.
+    if("openmp" IN_LIST LLVM_ENABLE_PROJECTS AND "offload" IN_LIST LLVM_ENABLE_RUNTIMES)
+      add_dependencies(clang-bootstrap-deps omp)
+    endif()
   endif()
 
   if(LLVM_INCLUDE_TESTS)
diff --git a/offload/CMakeLists.txt b/offload/CMakeLists.txt
index 42e0f5740f116d..3f77583ffa3b85 100644
--- a/offload/CMakeLists.txt
+++ b/offload/CMakeLists.txt
@@ -267,11 +267,6 @@ if(OPENMP_STANDALONE_BUILD)
       ${LLVM_LIBRARY_DIRS}
     REQUIRED
   )
-# Check LIBOMP_HAVE_VERSION_SCRIPT_FLAG
-  include(LLVMCheckCompilerLinkerFlag)
-  if(NOT APPLE)
-    llvm_check_compiler_linker_flag(C "-Wl,--version-script=${CMAKE_CURRENT_LIST_DIR}/../openmp/runtime/src/exports_test_so.txt" LIBOMP_HAVE_VERSION_SCRIPT_FLAG)
-  endif()
 
   macro(pythonize_bool var)
   if (${var})
@@ -282,6 +277,14 @@ if(OPENMP_STANDALONE_BUILD)
   endmacro()
 endif()
 
+if(OPENMP_STANDALONE_BUILD OR TARGET omp)
+  # Check LIBOMP_HAVE_VERSION_SCRIPT_FLAG
+  include(LLVMCheckCompilerLinkerFlag)
+  if(NOT APPLE)
+    llvm_check_compiler_linker_flag(C "-Wl,--version-script=${CMAKE_CURRENT_LIST_DIR}/../openmp/runtime/src/exports_test_so.txt" LIBOMP_HAVE_VERSION_SCRIPT_FLAG)
+  endif()
+endif()
+
 # OMPT support for libomptarget
 # Follow host OMPT support and check if host support has been requested.
 # LIBOMP_HAVE_OMPT_SUPPORT indicates whether host OMPT support has been implemented.

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems hacky, but what else is new.

@jhuber6 jhuber6 merged commit d824d87 into llvm:main May 2, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants