Skip to content

[Offload][NFCI] Remove coupling to omp target for version scripting #141637

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

Merged
merged 1 commit into from
May 27, 2025

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented May 27, 2025

Summary:
This is a weird dependency on libomp just for testing if version scripts
work. We shouldn't need to do this because LLVM already checks for
this. I believe this should be available as well in standalone when we
call addLLVM but I did not test that directly.

Summary:
This is a weird dependency on libomp just for testing if version scripts
work. We shouldn't need to do  this because LLVM already checks for
this. I believe this should be available as well in standalone when we
call `addLLVM` but I did not test that directly.
@llvmbot
Copy link
Member

llvmbot commented May 27, 2025

@llvm/pr-subscribers-offload

Author: Joseph Huber (jhuber6)

Changes

Summary:
This is a weird dependency on libomp just for testing if version scripts
work. We shouldn't need to do this because LLVM already checks for
this. I believe this should be available as well in standalone when we
call addLLVM but I did not test that directly.


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

3 Files Affected:

  • (modified) offload/CMakeLists.txt (-8)
  • (modified) offload/liboffload/CMakeLists.txt (+2-2)
  • (modified) offload/libomptarget/CMakeLists.txt (+2-3)
diff --git a/offload/CMakeLists.txt b/offload/CMakeLists.txt
index 2dc8285dd7730..c7cafd105f52a 100644
--- a/offload/CMakeLists.txt
+++ b/offload/CMakeLists.txt
@@ -322,14 +322,6 @@ else()
 endif()
 endmacro()
 
-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.
diff --git a/offload/liboffload/CMakeLists.txt b/offload/liboffload/CMakeLists.txt
index db12236ddfc7f..1b098bc01e218 100644
--- a/offload/liboffload/CMakeLists.txt
+++ b/offload/liboffload/CMakeLists.txt
@@ -14,8 +14,8 @@ foreach(plugin IN LISTS LIBOMPTARGET_PLUGINS_TO_BUILD)
     target_link_libraries(LLVMOffload PRIVATE omptarget.rtl.${plugin})
 endforeach()
 
-if(LIBOMP_HAVE_VERSION_SCRIPT_FLAG)
-    target_link_libraries(LLVMOffload PRIVATE "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/exports")
+if(LLVM_HAVE_LINK_VERSION_SCRIPT)
+  target_link_libraries(LLVMOffload PRIVATE "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/exports")
 endif()
 
 target_include_directories(LLVMOffload PUBLIC
diff --git a/offload/libomptarget/CMakeLists.txt b/offload/libomptarget/CMakeLists.txt
index c5f5d902fad14..93e684e53bf17 100644
--- a/offload/libomptarget/CMakeLists.txt
+++ b/offload/libomptarget/CMakeLists.txt
@@ -44,9 +44,8 @@ target_include_directories(omptarget PRIVATE
   ${LIBOMPTARGET_INCLUDE_DIR} ${LIBOMPTARGET_BINARY_INCLUDE_DIR}
 )
 
-if (LIBOMP_HAVE_VERSION_SCRIPT_FLAG)
-  target_link_libraries(omptarget PRIVATE
-    "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/exports")
+if(LLVM_HAVE_LINK_VERSION_SCRIPT)
+  target_link_libraries(omptarget PRIVATE "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/exports")
 endif()
 
 # Define the TARGET_NAME and DEBUG_PREFIX.

@jhuber6 jhuber6 merged commit 20f9f1f into llvm:main May 27, 2025
11 checks passed
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request May 28, 2025
…cripting (llvm#141637)"

breaks offload build

ld.lld: error: relocation R_X86_64_PC32 cannot be used against symbol 'ffi_type_float'; recompile with -fPIC
>>> defined in /usr/lib/x86_64-linux-gnu/libffi.a(types.o)

This reverts commit 20f9f1f.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…llvm#141637)

Summary:
This is a weird dependency on libomp just for testing if version scripts
work. We shouldn't need to do  this because LLVM already checks for
this. I believe this should be available as well in standalone when we
call `addLLVM` but I did not test that directly.
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.

3 participants