Skip to content

[Offload] Remove unnecessary omp CMake target dependencies #149060

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

Closed

Conversation

kbenzie
Copy link
Contributor

@kbenzie kbenzie commented Jul 16, 2025

In my quest to build liboffload with ASAN enabled I've been looking into decoupling offload and openmp project entablement. When removing openmp from LLVM_ENABLE_PROJECTS it resulted in failure to link against the omp target. As an experiment I disabled that dependency and the build appears to work as expected.

This patch removes dependencies on the omp target from libomptarget/llvm-offload-device-info/llvm-omp-kernel-replay targets.

@kbenzie kbenzie force-pushed the benie/remove-libomptarget-omp-dependency branch from ac4d5f0 to bc38c06 Compare July 16, 2025 10:35
@kbenzie kbenzie marked this pull request as ready for review July 16, 2025 10:42
@llvmbot
Copy link
Member

llvmbot commented Jul 16, 2025

@llvm/pr-subscribers-offload

Author: Kenneth Benzie (Benie) (kbenzie)

Changes

In my quest to build liboffload with ASAN enabled I've been looking into decoupling offload and openmp project entablement. When removing openmp from LLVM_ENABLE_PROJECTS it resulted in failure to link against the omp target. As an experiment I disabled that dependency and the build appears to work as expect.

This patch removes dependencies on the omp target from libomptarget/llvm-offload-device-info/llvm-omp-kernel-replay targets.


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

3 Files Affected:

  • (modified) offload/libomptarget/CMakeLists.txt (-10)
  • (modified) offload/tools/deviceinfo/CMakeLists.txt (-1)
  • (modified) offload/tools/kernelreplay/CMakeLists.txt (-1)
diff --git a/offload/libomptarget/CMakeLists.txt b/offload/libomptarget/CMakeLists.txt
index 93e684e53bf17..f37e984504167 100644
--- a/offload/libomptarget/CMakeLists.txt
+++ b/offload/libomptarget/CMakeLists.txt
@@ -1,11 +1,5 @@
 message(STATUS "Building offloading runtime library libomptarget.")
 
-if(LIBOMP_STANDALONE)
-  set(LIBOMP ${LIBOMP_STANDALONE})
-else()
-  set(LIBOMP omp)
-endif()
-
 add_llvm_library(omptarget
   SHARED
 
@@ -33,10 +27,6 @@ add_llvm_library(omptarget
   Support
   Object
 
-  LINK_LIBS
-  PUBLIC
-  ${LIBOMP}
-
   NO_INSTALL_RPATH
   BUILDTREE_ONLY
 )
diff --git a/offload/tools/deviceinfo/CMakeLists.txt b/offload/tools/deviceinfo/CMakeLists.txt
index 3787c12f940a6..6edc4f057c8c9 100644
--- a/offload/tools/deviceinfo/CMakeLists.txt
+++ b/offload/tools/deviceinfo/CMakeLists.txt
@@ -8,6 +8,5 @@ target_include_directories(llvm-offload-device-info PRIVATE
   ${LIBOMPTARGET_INCLUDE_DIR}
 )
 target_link_libraries(llvm-offload-device-info PRIVATE
-  omp
   omptarget
 )
diff --git a/offload/tools/kernelreplay/CMakeLists.txt b/offload/tools/kernelreplay/CMakeLists.txt
index 690edd6d5a68e..931b1947fffb4 100644
--- a/offload/tools/kernelreplay/CMakeLists.txt
+++ b/offload/tools/kernelreplay/CMakeLists.txt
@@ -9,6 +9,5 @@ target_include_directories(llvm-omp-kernel-replay PRIVATE
 )
 target_link_libraries(llvm-omp-kernel-replay PRIVATE
   LLVMSupport
-  omp
   omptarget
 )

In my quest to build `liboffload` with ASAN enabled I've been looking
into decoupling `offload` and `openmp` project entablement. When
removing `openmp` from `LLVM_ENABLE_PROJECTS` it resulted in failure to
link against the `omp` target. As an experiment I disabled that
dependency and the build appears to work as expected.

This patch removes dependencies on the `omp` target from
`libomptarget`/`llvm-offload-device-info`/`llvm-omp-kernel-replay`
targets.
@kbenzie kbenzie force-pushed the benie/remove-libomptarget-omp-dependency branch from bc38c06 to b330783 Compare July 16, 2025 11:25
@shiltian
Copy link
Contributor

IIRC there are indeed some function calls from libomptarget to libomp.

@jhuber6
Copy link
Contributor

jhuber6 commented Jul 16, 2025

IIRC there are indeed some function calls from libomptarget to libomp.

Yeah, there are a few but I forget which ones. Did this really build and pass the tests?

@kbenzie
Copy link
Contributor Author

kbenzie commented Jul 16, 2025

IIRC there are indeed some function calls from libomptarget to libomp.

Yeah, there are a few but I forget which ones. Did this really build and pass the tests?

The build works with and without openmp in LLVM_ENABLE_RUNTIMES. I'll admit I don't have a fully functional OpenMP test environment set up currently so I'm not sure about that.

Would libomptarget be dynamically loading symbols from libomp at all?
If not, I'm not sure I understand how the build can be successful if there are actually uses of symbols from that library.

@jhuber6
Copy link
Contributor

jhuber6 commented Jul 16, 2025

Yes, there's some libomp functions that libomptarget calls. omp_get_initial_device is an example. This is part of why I feel libomptarget should be moved out of offload and back into openmp.

@kbenzie
Copy link
Contributor Author

kbenzie commented Jul 16, 2025

Is #136729 related to that?

I'm wondering if it would it be possible to move the implementations of these libomp entry points into libomptarget and have libomp be implemented in terms of libomptarget rather than this circular dependency situation?

@jhuber6
Copy link
Contributor

jhuber6 commented Jul 16, 2025

Is #136729 related to that?

I'm wondering if it would it be possible to move the implementations of these libomp entry points into libomptarget and have libomp be implemented in terms of libomptarget rather than this circular dependency situation?

Yes, it's related but only moves out the OpenMP device library. What we should do is re-write libomptarget's plugin layer to use the newly created API and then move all the code into openmp. Then we just link the offloading runtime when we build.

@shiltian
Copy link
Contributor

It works because a program always links libomp and libomptarget but we can't say there is no need for a dependency between libomptarget and libomp.

@shiltian
Copy link
Contributor

IIRC there are indeed some function calls from libomptarget to libomp.

Yeah, there are a few but I forget which ones. Did this really build and pass the tests?

Those nowait handling related stuff also depends on libomp.

@kbenzie
Copy link
Contributor Author

kbenzie commented Jul 17, 2025

Okay, thanks for the info. I'll close this.

@kbenzie kbenzie closed this Jul 17, 2025
@kbenzie kbenzie deleted the benie/remove-libomptarget-omp-dependency branch July 17, 2025 08:54
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.

4 participants