Skip to content

Error out when LIBOMPTARGET_OMPT_SUPPORT is requested but not supported #146865

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

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()

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