[Offload] Implement 'olIsValidBinary' in offload and clean up #159658

jhuber6 · 2025-09-18T21:47:51Z

Summary:
This exposes the 'isDeviceCompatible' routine for checking if a binary
can be loaded. This is useful if people don't want to consume errors
everywhere when figuring out which image to put to what device.

I don't know if this is a good name, I was thining like olIsCompatible
or whatever. Let me know what you think.

Long term I'd like to be able to do something similar to what OpenMP
does where we can conditionally only initialize devices if we need them.
That's going to be support needed if we want this to be more
generic.

jhuber6 · 2025-09-18T21:48:04Z

This is similar to #156259

llvmbot · 2025-09-18T21:48:21Z

@llvm/pr-subscribers-offload

Author: Joseph Huber (jhuber6)

Changes

Summary:
This exposes the 'isDeviceCompatible' routine for checking if a binary
can be loaded. This is useful if people don't want to consume errors
everywhere when figuring out which image to put to what device.

I don't know if this is a good name, I was thining like olIsCompatible
or whatever. Let me know what you think.

Long term I'd like to be able to do something similar to what OpenMP
does where we can conditionally only initialize devices if we need them.
That's going to be support needed if we want this to be more
generic.

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

8 Files Affected:

(modified) offload/liboffload/API/Common.td (-1)
(modified) offload/liboffload/API/Program.td (+12)
(modified) offload/liboffload/src/OffloadImpl.cpp (+9)
(modified) offload/libomptarget/PluginManager.cpp (+5-2)
(modified) offload/plugins-nextgen/common/include/PluginInterface.h (+2-2)
(modified) offload/plugins-nextgen/common/src/PluginInterface.cpp (+10-16)
(modified) offload/unittests/OffloadAPI/CMakeLists.txt (+1)
(added) offload/unittests/OffloadAPI/program/olValidateBinary.cpp (+26)

diff --git a/offload/liboffload/API/Common.td b/offload/liboffload/API/Common.td
index ac27d85b6c964..1af6527f6ac86 100644
--- a/offload/liboffload/API/Common.td
+++ b/offload/liboffload/API/Common.td
@@ -91,7 +91,6 @@ def ol_errc_t : Enum {
     Etor<"LINK_FAILURE", "linker failure while processing binary image">,
     Etor<"BACKEND_FAILURE", "the plugin backend is in an invalid or unsupported state">,
     Etor<"UNINITIALIZED", "not initialized">,
-
     // Handle related errors - only makes sense for liboffload
     Etor<"INVALID_NULL_HANDLE", "a handle argument is null when it should not be">,
     Etor<"INVALID_PLATFORM", "invalid platform">,
diff --git a/offload/liboffload/API/Program.td b/offload/liboffload/API/Program.td
index 1f48f650cab70..fbb48bf76fcc1 100644
--- a/offload/liboffload/API/Program.td
+++ b/offload/liboffload/API/Program.td
@@ -24,6 +24,18 @@ def olCreateProgram : Function {
     let returns = [];
 }
 
+def olValidateBinary : Function {
+    let desc = "Validate if the binary image pointed to by `ProgData` is compatible with the device.";
+    let details = ["The provided `ProgData` will not be loaded onto the device"];
+    let params = [
+        Param<"ol_device_handle_t", "Device", "handle of the device", PARAM_IN>,
+        Param<"const void*", "ProgData", "pointer to the program binary data", PARAM_IN>,
+        Param<"size_t", "ProgDataSize", "size of the program binary in bytes", PARAM_IN>,
+        Param<"bool*", "Valid", "output is true if the image is compatible", PARAM_OUT>
+    ];
+    let returns = [];
+}
+
 def olDestroyProgram : Function {
     let desc = "Destroy the program and free all underlying resources.";
     let details = [];
diff --git a/offload/liboffload/src/OffloadImpl.cpp b/offload/liboffload/src/OffloadImpl.cpp
index b5b9b0e83b975..2c08e37a96e9a 100644
--- a/offload/liboffload/src/OffloadImpl.cpp
+++ b/offload/liboffload/src/OffloadImpl.cpp
@@ -899,6 +899,15 @@ Error olCreateProgram_impl(ol_device_handle_t Device, const void *ProgData,
   return Error::success();
 }
 
+Error olValidateBinary_impl(ol_device_handle_t Device, const void *ProgData,
+                            size_t ProgDataSize, bool *IsValid) {
+  // Make a copy of the program binary in case it is released by the caller.
+  StringRef Buffer(reinterpret_cast<const char *>(ProgData), ProgDataSize);
+  *IsValid = Device->Device->Plugin.isDeviceCompatible(
+      Device->Device->getDeviceId(), Buffer);
+  return Error::success();
+}
+
 Error olDestroyProgram_impl(ol_program_handle_t Program) {
   auto &Device = Program->Image->getDevice();
   if (auto Err = Device.unloadBinary(Program->Image))
diff --git a/offload/libomptarget/PluginManager.cpp b/offload/libomptarget/PluginManager.cpp
index b57a2f815cba6..c8d6b42114d0f 100644
--- a/offload/libomptarget/PluginManager.cpp
+++ b/offload/libomptarget/PluginManager.cpp
@@ -219,7 +219,10 @@ void PluginManager::registerLib(__tgt_bin_desc *Desc) {
     // Scan the RTLs that have associated images until we find one that supports
     // the current image.
     for (auto &R : plugins()) {
-      if (!R.is_plugin_compatible(Img))
+      StringRef Buffer(reinterpret_cast<const char *>(Img->ImageStart),
+                       utils::getPtrDiff(Img->ImageEnd, Img->ImageStart));
+
+      if (!R.isPluginCompatible(Buffer))
         continue;
 
       if (!initializePlugin(R))
@@ -242,7 +245,7 @@ void PluginManager::registerLib(__tgt_bin_desc *Desc) {
           continue;
         }
 
-        if (!R.is_device_compatible(DeviceId, Img))
+        if (!R.isDeviceCompatible(DeviceId, Buffer))
           continue;
 
         DP("Image " DPxMOD " is compatible with RTL %s device %d!\n",
diff --git a/offload/plugins-nextgen/common/include/PluginInterface.h b/offload/plugins-nextgen/common/include/PluginInterface.h
index ce66d277d6187..9d5651a3d7b4e 100644
--- a/offload/plugins-nextgen/common/include/PluginInterface.h
+++ b/offload/plugins-nextgen/common/include/PluginInterface.h
@@ -1378,10 +1378,10 @@ struct GenericPluginTy {
 
   /// Returns non-zero if the \p Image is compatible with the plugin. This
   /// function does not require the plugin to be initialized before use.
-  int32_t is_plugin_compatible(__tgt_device_image *Image);
+  int32_t isPluginCompatible(StringRef Image);
 
   /// Returns non-zero if the \p Image is compatible with the device.
-  int32_t is_device_compatible(int32_t DeviceId, __tgt_device_image *Image);
+  int32_t isDeviceCompatible(int32_t DeviceId, StringRef Image);
 
   /// Returns non-zero if the plugin device has been initialized.
   int32_t is_device_initialized(int32_t DeviceId) const;
diff --git a/offload/plugins-nextgen/common/src/PluginInterface.cpp b/offload/plugins-nextgen/common/src/PluginInterface.cpp
index 9f830874d5dad..d2078a2775791 100644
--- a/offload/plugins-nextgen/common/src/PluginInterface.cpp
+++ b/offload/plugins-nextgen/common/src/PluginInterface.cpp
@@ -1713,28 +1713,25 @@ Expected<bool> GenericPluginTy::checkBitcodeImage(StringRef Image) const {
 
 int32_t GenericPluginTy::is_initialized() const { return Initialized; }
 
-int32_t GenericPluginTy::is_plugin_compatible(__tgt_device_image *Image) {
-  StringRef Buffer(reinterpret_cast<const char *>(Image->ImageStart),
-                   utils::getPtrDiff(Image->ImageEnd, Image->ImageStart));
-
+int32_t GenericPluginTy::isPluginCompatible(StringRef Image) {
   auto HandleError = [&](Error Err) -> bool {
     [[maybe_unused]] std::string ErrStr = toString(std::move(Err));
     DP("Failure to check validity of image %p: %s", Image, ErrStr.c_str());
     return false;
   };
-  switch (identify_magic(Buffer)) {
+  switch (identify_magic(Image)) {
   case file_magic::elf:
   case file_magic::elf_relocatable:
   case file_magic::elf_executable:
   case file_magic::elf_shared_object:
   case file_magic::elf_core: {
-    auto MatchOrErr = checkELFImage(Buffer);
+    auto MatchOrErr = checkELFImage(Image);
     if (Error Err = MatchOrErr.takeError())
       return HandleError(std::move(Err));
     return *MatchOrErr;
   }
   case file_magic::bitcode: {
-    auto MatchOrErr = checkBitcodeImage(Buffer);
+    auto MatchOrErr = checkBitcodeImage(Image);
     if (Error Err = MatchOrErr.takeError())
       return HandleError(std::move(Err));
     return *MatchOrErr;
@@ -1744,36 +1741,33 @@ int32_t GenericPluginTy::is_plugin_compatible(__tgt_device_image *Image) {
   }
 }
 
-int32_t GenericPluginTy::is_device_compatible(int32_t DeviceId,
-                                              __tgt_device_image *Image) {
-  StringRef Buffer(reinterpret_cast<const char *>(Image->ImageStart),
-                   utils::getPtrDiff(Image->ImageEnd, Image->ImageStart));
-
+int32_t GenericPluginTy::isDeviceCompatible(int32_t DeviceId,
+                                              StringRef Image) {
   auto HandleError = [&](Error Err) -> bool {
     [[maybe_unused]] std::string ErrStr = toString(std::move(Err));
     DP("Failure to check validity of image %p: %s", Image, ErrStr.c_str());
     return false;
   };
-  switch (identify_magic(Buffer)) {
+  switch (identify_magic(Image)) {
   case file_magic::elf:
   case file_magic::elf_relocatable:
   case file_magic::elf_executable:
   case file_magic::elf_shared_object:
   case file_magic::elf_core: {
-    auto MatchOrErr = checkELFImage(Buffer);
+    auto MatchOrErr = checkELFImage(Image);
     if (Error Err = MatchOrErr.takeError())
       return HandleError(std::move(Err));
     if (!*MatchOrErr)
       return false;
 
     // Perform plugin-dependent checks for the specific architecture if needed.
-    auto CompatibleOrErr = isELFCompatible(DeviceId, Buffer);
+    auto CompatibleOrErr = isELFCompatible(DeviceId, Image);
     if (Error Err = CompatibleOrErr.takeError())
       return HandleError(std::move(Err));
     return *CompatibleOrErr;
   }
   case file_magic::bitcode: {
-    auto MatchOrErr = checkBitcodeImage(Buffer);
+    auto MatchOrErr = checkBitcodeImage(Image);
     if (Error Err = MatchOrErr.takeError())
       return HandleError(std::move(Err));
     return *MatchOrErr;
diff --git a/offload/unittests/OffloadAPI/CMakeLists.txt b/offload/unittests/OffloadAPI/CMakeLists.txt
index b2d514423a6ee..ad5592a1b4d28 100644
--- a/offload/unittests/OffloadAPI/CMakeLists.txt
+++ b/offload/unittests/OffloadAPI/CMakeLists.txt
@@ -35,6 +35,7 @@ add_offload_unittest("platform"
 
 add_offload_unittest("program"
     program/olCreateProgram.cpp
+    program/olValidateBinary.cpp
     program/olDestroyProgram.cpp)
 
 add_offload_unittest("queue"
diff --git a/offload/unittests/OffloadAPI/program/olValidateBinary.cpp b/offload/unittests/OffloadAPI/program/olValidateBinary.cpp
new file mode 100644
index 0000000000000..63d2eba7092c5
--- /dev/null
+++ b/offload/unittests/OffloadAPI/program/olValidateBinary.cpp
@@ -0,0 +1,26 @@
+//===------- Offload API tests - oldValidateBinary ------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "../common/Fixtures.hpp"
+#include <OffloadAPI.h>
+#include <gtest/gtest.h>
+
+using olValidateBinaryTest = OffloadDeviceTest;
+OFFLOAD_TESTS_INSTANTIATE_DEVICE_FIXTURE(olValidateBinaryTest);
+
+TEST_P(olValidateBinaryTest, Success) {
+
+  std::unique_ptr<llvm::MemoryBuffer> DeviceBin;
+  ASSERT_TRUE(TestEnvironment::loadDeviceBinary("foo", Device, DeviceBin));
+  ASSERT_GE(DeviceBin->getBufferSize(), 0lu);
+
+  bool IsValid = false;
+  ASSERT_SUCCESS(olValidateBinary(Device, DeviceBin->getBufferStart(),
+                                  DeviceBin->getBufferSize(), &IsValid));
+  ASSERT_TRUE(IsValid);
+}

github-actions · 2025-09-18T22:43:50Z

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

Summary: This exposes the 'isDeviceCompatible' routine for checking if a binary can be loaded. This is useful if people don't want to consume errors everywhere when figuring out which image to put to what device. I don't know if this is a good name, I was thining like olIsCompatible or whatever. Let me know what you think. Long term I'd like to be able to do something similar to what OpenMP does where we can conditionally only initialize devices if we need them. That's going to be support needed if we want this to be more generic.

RossBrunton · 2025-09-19T10:17:46Z

offload/liboffload/API/Program.td

    let returns = [];
 }

+def olIsValidBinary : Function {


I'd prefer olIsCompatibleBinary; an ELF file (for example) can be a perfectly valid file yet not be compatible with the device.

RossBrunton · 2025-09-19T10:21:50Z

offload/liboffload/src/OffloadImpl.cpp


+Error olIsValidBinary_impl(ol_device_handle_t Device, const void *ProgData,
+                           size_t ProgDataSize, bool *IsValid) {
+  // Make a copy of the program binary in case it is released by the caller.


This can only happen if the caller frees it from another thread while we are calling this, right? I think that's an obnoxious enough thing to do that we should probably just call it UB. We have lots of other functions elsewhere that assume their arguments are not free'd during the call.

Oh I forgot to delete that comment, it's not even true anymore from where it was copied.

offload/unittests/OffloadAPI/program/olIsValidBinary.cpp

jhuber6 · 2025-09-19T14:28:41Z

Also @RossBrunton, this isn't related at all to the patch, but I'm wondering what you think about #159636. The problem is that initializing all devices and plugins is quite expensive when multiple are available, it takes something like 250 ms to initialize CUDA when I don't intend to use it for example.

Do you have any good ideas for possibly relaxing this? Maybe initialize devices and plugins at first use? It's a little tough because I think we discussed previously that the platform is kind of an implementation detail of a device. We could possibly make it more explicit that users need.

I believe in most APIs you get to iterate the devices without necessarily initializing them for use. Another issue is that we need to initialize all of CUDA / HSA to iterate them in the first place. Any good ideas? I don't know the internal mechanisms of the runtime as much.

…llvm#159658)" This reverts commit 51e3c3d.

…59658) Summary: This exposes the 'isDeviceCompatible' routine for checking if a binary *can* be loaded. This is useful if people don't want to consume errors everywhere when figuring out which image to put to what device. I don't know if this is a good name, I was thining like `olIsCompatible` or whatever. Let me know what you think. Long term I'd like to be able to do something similar to what OpenMP does where we can conditionally only initialize devices if we need them. That's going to be support needed if we want this to be more generic.

jhuber6 requested review from RossBrunton, callumfare, jplehr, ronlieb and shiltian September 18, 2025 21:47

llvmbot added the offload label Sep 18, 2025

jhuber6 force-pushed the Compatible branch 2 times, most recently from 3587602 to f47951b Compare September 18, 2025 22:41

jhuber6 changed the title ~~[Offload] Implement 'olValidateBinary' in offload and clean up~~ [Offload] Implement 'olIsValidBinary' in offload and clean up Sep 18, 2025

jhuber6 force-pushed the Compatible branch from f47951b to cd5a0ad Compare September 18, 2025 22:44

shiltian approved these changes Sep 19, 2025

View reviewed changes

RossBrunton reviewed Sep 19, 2025

View reviewed changes

offload/unittests/OffloadAPI/program/olIsValidBinary.cpp Show resolved Hide resolved

jhuber6 added 2 commits September 19, 2025 07:50

More test

915c4ac

remove incorrect comment

a241dd9

RossBrunton reviewed Sep 19, 2025

View reviewed changes

offload/unittests/OffloadAPI/program/olIsValidBinary.cpp Show resolved Hide resolved

better test

1726cfd

jhuber6 merged commit 51e3c3d into llvm:main Sep 19, 2025
9 checks passed

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Sep 20, 2025

Revert "[Offload] Implement 'olIsValidBinary' in offload and clean up (…

9cd428e

…llvm#159658)" This reverts commit 51e3c3d.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Offload] Implement 'olIsValidBinary' in offload and clean up #159658

[Offload] Implement 'olIsValidBinary' in offload and clean up #159658

Uh oh!

jhuber6 commented Sep 18, 2025

Uh oh!

jhuber6 commented Sep 18, 2025

Uh oh!

llvmbot commented Sep 18, 2025

Uh oh!

github-actions bot commented Sep 18, 2025 •

edited

Loading

Uh oh!

RossBrunton Sep 19, 2025

Uh oh!

RossBrunton Sep 19, 2025

Uh oh!

jhuber6 Sep 19, 2025

Uh oh!

Uh oh!

Uh oh!

jhuber6 commented Sep 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[Offload] Implement 'olIsValidBinary' in offload and clean up #159658

[Offload] Implement 'olIsValidBinary' in offload and clean up #159658

Uh oh!

Conversation

jhuber6 commented Sep 18, 2025

Uh oh!

jhuber6 commented Sep 18, 2025

Uh oh!

llvmbot commented Sep 18, 2025

Uh oh!

github-actions bot commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RossBrunton Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

RossBrunton Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

jhuber6 Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jhuber6 commented Sep 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Sep 18, 2025 •

edited

Loading