Skip to content

Conversation

@adurang
Copy link
Contributor

@adurang adurang commented Mar 28, 2025

If user specifies offload is disabled (e.g., OMP_TARGET_OFFLOAD=disable), disable library almost completely. This reduces resources spent to a minimum and ensures all APIs behave as if the only available device is the host device.

Currently some of the APIs behave as if there were devices avaible for offload even when under OMP_TARGET_OFFLOAD=disable.

If user specifies offload is disabled (e.g., OMP_TARGET_OFFLOAD=disable),
disable library almost completely. This reduces resources spent
to a minimum and ensures all APIs behave as if the only available device
is the host device.

Currently some of the APIs behave as if there were devices
avaible for offload even when under OMP_TARGET_OFFLOAD=disable.
@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2025

@llvm/pr-subscribers-offload

Author: Alex (adurang)

Changes

If user specifies offload is disabled (e.g., OMP_TARGET_OFFLOAD=disable), disable library almost completely. This reduces resources spent to a minimum and ensures all APIs behave as if the only available device is the host device.

Currently some of the APIs behave as if there were devices avaible for offload even when under OMP_TARGET_OFFLOAD=disable.


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

2 Files Affected:

  • (modified) offload/include/OffloadPolicy.h (+20-2)
  • (modified) offload/libomptarget/OffloadRTL.cpp (+7)
diff --git a/offload/include/OffloadPolicy.h b/offload/include/OffloadPolicy.h
index 858d9c323b15a..7f57d4da8b364 100644
--- a/offload/include/OffloadPolicy.h
+++ b/offload/include/OffloadPolicy.h
@@ -26,7 +26,7 @@ extern "C" int __kmpc_get_target_offload(void) __attribute__((weak));
 
 class OffloadPolicy {
 
-  OffloadPolicy(PluginManager &PM) {
+  OffloadPolicy() {
     // TODO: Check for OpenMP.
     switch ((kmp_target_offload_kind_t)__kmpc_get_target_offload()) {
     case tgt_disabled:
@@ -36,6 +36,17 @@ class OffloadPolicy {
       Kind = MANDATORY;
       return;
     default:
+      // delay DEFAULT policy until PluginManager is ready
+      UserValue = false;
+      return;
+    };
+  }
+
+  OffloadPolicy(PluginManager &PM) {
+    const OffloadPolicy &OP = get();
+    if (!OP.UserValue) {
+      // User didn't specify a policy, decide
+      // based on number of devices discovered
       if (PM.getNumDevices()) {
         DP("Default TARGET OFFLOAD policy is now mandatory "
            "(devices were found)\n");
@@ -46,10 +57,16 @@ class OffloadPolicy {
         Kind = DISABLED;
       }
       return;
-    };
+    }
+    Kind = OP.Kind;
   }
 
 public:
+  static const OffloadPolicy &get() {
+    static OffloadPolicy OP;
+    return OP;
+  }
+
   static const OffloadPolicy &get(PluginManager &PM) {
     static OffloadPolicy OP(PM);
     return OP;
@@ -58,6 +75,7 @@ class OffloadPolicy {
   enum OffloadPolicyKind { DISABLED, MANDATORY };
 
   OffloadPolicyKind Kind = MANDATORY;
+  bool UserValue = true;
 };
 
 #endif // OMPTARGET_OFFLOAD_POLICY_H
diff --git a/offload/libomptarget/OffloadRTL.cpp b/offload/libomptarget/OffloadRTL.cpp
index 29b573a27d087..ecf7b501efcbc 100644
--- a/offload/libomptarget/OffloadRTL.cpp
+++ b/offload/libomptarget/OffloadRTL.cpp
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "OffloadPolicy.h"
 #include "OpenMP/OMPT/Callback.h"
 #include "PluginManager.h"
 
@@ -31,6 +32,12 @@ void initRuntime() {
   if (PM == nullptr)
     PM = new PluginManager();
 
+  if (OffloadPolicy::get().Kind == OffloadPolicy::DISABLED) {
+    DP("Offload is disabled. Skipping library initialization\n");
+    // Do only absolutely needed initialization
+    return;
+  }
+
   RefCount++;
   if (RefCount == 1) {
     DP("Init offload library!\n");

@github-actions
Copy link

github-actions bot commented Mar 28, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

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.

We should just omit calling PM->init() (Possibly just an early exit in init() itself. Since offloading is disabled we need no plugins so it's logical).

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.

LG, thanks.

@jhuber6 jhuber6 merged commit 021a1f6 into llvm:main Mar 28, 2025
9 checks passed
@adurang
Copy link
Contributor Author

adurang commented Mar 29, 2025

thank you @jhuber6!

mhalk added a commit to mhalk/llvm-project that referenced this pull request Apr 2, 2025
Add early exit during plugin de-init
 - when OffloadPolicy::isOffloadDisabled
 - e.g.: `OMP_TARGET_OFFLOAD=DISABLED`

See also: llvm#133470
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