Skip to content

Conversation

@Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Jul 3, 2025

Currently a user could pass -DLIBOMPTARGET_OMPT_SUPPORT=ON but the build may ignore it

There is already a check for LIBOMP_OMPT_SUPPORT AND NOT LIBOMP_HAVE_OMPT_SUPPORT causing an error so a) There should be a similar error on mismatch of passed options and supported configuration b) The check for LIBOMP_HAVE_OMPT_SUPPORT is redundant

We ran into this for automated builds where the output "OMPT target disabled" isn't as useful as a hard abort (exit code !=0)

@llvmbot llvmbot added the offload label Jul 3, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 3, 2025

@llvm/pr-subscribers-offload

Author: Alexander Grund (Flamefire)

Changes

Currently a user could pass -DLIBOMPTARGET_OMPT_SUPPORT=ON but the build may ignore it

There is already a check for LIBOMP_OMPT_SUPPORT AND NOT LIBOMP_HAVE_OMPT_SUPPORT causing an error so a) There should be a similar error on mismatch of passed options and supported configuration b) The check for LIBOMP_HAVE_OMPT_SUPPORT is redundant


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

1 Files Affected:

  • (modified) offload/CMakeLists.txt (+6-4)
diff --git a/offload/CMakeLists.txt b/offload/CMakeLists.txt
index 0a441c3bc5782..f5c39e9ebf4a3 100644
--- a/offload/CMakeLists.txt
+++ b/offload/CMakeLists.txt
@@ -326,15 +326,17 @@ endmacro()
 
 # 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.
 # LIBOMP_OMPT_SUPPORT indicates whether host OMPT support has been requested (default is ON).
-# LIBOMPTARGET_OMPT_SUPPORT indicates whether target OMPT support has been requested (default is ON).
 set(OMPT_TARGET_DEFAULT FALSE)
-if ((LIBOMP_HAVE_OMPT_SUPPORT) AND (LIBOMP_OMPT_SUPPORT) AND (NOT WIN32))
+if (LIBOMP_OMPT_SUPPORT AND (NOT WIN32))
   set (OMPT_TARGET_DEFAULT TRUE)
 endif()
+# LIBOMPTARGET_OMPT_SUPPORT indicates whether target OMPT support has been requested.
 set(LIBOMPTARGET_OMPT_SUPPORT ${OMPT_TARGET_DEFAULT} CACHE BOOL "OMPT-target-support?")
-if ((OMPT_TARGET_DEFAULT) AND (LIBOMPTARGET_OMPT_SUPPORT))
+if (LIBOMPTARGET_OMPT_SUPPORT)
+  if(NOT LIBOMP_OMPT_SUPPORT)
+    message(FATAL_ERROR "OMPT Target support requested but OMPT (host) support is not enabled")
+  endif()
   add_definitions(-DOMPT_SUPPORT=1)
   message(STATUS "OMPT target enabled")
 else()

@github-actions
Copy link

github-actions bot commented Jul 3, 2025

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Developer Policy and LLVM Discourse for more information.

…rted

Currently a user could pass `-DLIBOMPTARGET_OMPT_SUPPORT=ON` but the build may ignore it

There is already a check for `LIBOMP_OMPT_SUPPORT AND NOT LIBOMP_HAVE_OMPT_SUPPORT` causing an error so
a) There should be a similar error on mismatch of passed options and supported configuration
b) The check for `LIBOMP_HAVE_OMPT_SUPPORT` is redundant
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.

2 participants