Skip to content

Conversation

@RossBrunton
Copy link
Contributor

This wasn't handled with the normal info API, so needs special handling.

This wasn't handled with the normal info API, so needs special handling.
@llvmbot
Copy link
Member

llvmbot commented Aug 20, 2025

@llvm/pr-subscribers-offload

Author: Ross Brunton (RossBrunton)

Changes

This wasn't handled with the normal info API, so needs special handling.


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

1 Files Affected:

  • (modified) offload/liboffload/src/OffloadImpl.cpp (+9-10)
diff --git a/offload/liboffload/src/OffloadImpl.cpp b/offload/liboffload/src/OffloadImpl.cpp
index 54c6d452cbd9e..5374b87ee63f6 100644
--- a/offload/liboffload/src/OffloadImpl.cpp
+++ b/offload/liboffload/src/OffloadImpl.cpp
@@ -339,8 +339,15 @@ Error olGetDeviceInfoImplDetail(ol_device_handle_t Device,
   // None of the existing plugins specify a limit on a single allocation,
   // so return the global memory size instead
   case OL_DEVICE_INFO_MAX_MEM_ALLOC_SIZE:
-    PropName = OL_DEVICE_INFO_GLOBAL_MEM_SIZE;
-    break;
+    [[fallthrough]];
+  // AMD doesn't provide the global memory size (trivially) with the device info
+  // struct, so use the plugin interface
+  case OL_DEVICE_INFO_GLOBAL_MEM_SIZE: {
+    uint64_t Mem;
+    if (auto Err = Device->Device->getDeviceMemorySize(Mem))
+      return Err;
+    return Info.write<uint64_t>(Mem);
+  } break;
 
   default:
     break;
@@ -368,14 +375,6 @@ Error olGetDeviceInfoImplDetail(ol_device_handle_t Device,
     return Info.writeString(std::get<std::string>(Entry->Value).c_str());
   }
 
-  case OL_DEVICE_INFO_GLOBAL_MEM_SIZE: {
-    // Uint64 values
-    if (!std::holds_alternative<uint64_t>(Entry->Value))
-      return makeError(ErrorCode::BACKEND_FAILURE,
-                       "plugin returned incorrect type");
-    return Info.write(std::get<uint64_t>(Entry->Value));
-  }
-
   case OL_DEVICE_INFO_MAX_WORK_GROUP_SIZE:
   case OL_DEVICE_INFO_VENDOR_ID:
   case OL_DEVICE_INFO_NUM_COMPUTE_UNITS:

@RossBrunton
Copy link
Contributor Author

@rafbiels Ping. Just a small fix to this because it wasn't working on AMD.

@RossBrunton RossBrunton merged commit 273ca1f into llvm:main Aug 21, 2025
11 checks passed
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.

3 participants