[OFFLOAD] Add missing platform properties for libomptarget migration #171009

fineg74 · 2025-12-07T04:18:15Z

Add missing liboffload platform properties for libomptarget migration

This PR adds liboffload platform properties that needed to make libomptarget to use liboffload

github-actions · 2025-12-07T04:18:30Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

sarnex

i don't know anything about liboffload or libomptarget so i can't offer any useful feedback, only generic comments

sarnex · 2025-12-08T20:24:07Z

offload/liboffload/API/Device.td

    TaggedEtor<"MAX_MEM_ALLOC_SIZE", "uint64_t", "The maximum size of memory object allocation in bytes">,
    TaggedEtor<"GLOBAL_MEM_SIZE", "uint64_t", "The size of global device memory in bytes">,
    TaggedEtor<"WORK_GROUP_LOCAL_MEM_SIZE", "uint64_t", "The maximum size of local shared memory per work group in bytes">,
+    TaggedEtor<"ID", "int32_t", "Device ID">,


can we not use UID?

sarnex · 2025-12-08T20:24:49Z

offload/liboffload/API/Device.td

+}
+
+def olSelectInterop : Function {
+  let desc = "Return OpenMP interop object that the device supports";


nit: it seems all the descriptions should end with a period

sarnex · 2025-12-08T20:26:22Z

offload/liboffload/API/Queue.td

+        Param<"ol_queue_handle_t", "Queue", "handle of the queue", PARAM_IN>
+    ];
+    let returns = [];
+}


nit newline at eof

sarnex · 2025-12-08T20:28:33Z

offload/liboffload/src/OffloadImpl.cpp


+Error olDataFence_impl(ol_queue_handle_t Queue) {
+  if (Queue->AsyncInfo->Queue) {
+    if (auto Err = Queue->Device->Device->dataFence(Queue->AsyncInfo))


don't think it's related to this PR but Device->Device->fcn() is kinda weird :P

It is pretty common in liboffload code: bunch of classes have members called Device although in different classes they mean different things (liboffload Device type vs GenericDeviceTy from libomptarget). BTW, there is similar issue in libomptarget as well where we have different device types that abstract actual device on different API levels.

sarnex · 2025-12-08T20:30:01Z

offload/liboffload/src/OffloadImpl.cpp

+
+Error olSelectInterop_impl(ol_device_handle_t Device, int32_t InteropType, int32_t InteropPreferencesSize, 
+        void *InteropPreferences, void* InteropSpec) {
+  *(interop_spec_t*)InteropSpec = Device->Device->selectInteropPreference(


nit: can we static_cast instead of C cast? It's a bit safer.

github-actions · 2025-12-08T20:32:25Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff origin/main HEAD --extensions cpp -- offload/liboffload/src/OffloadImpl.cpp --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.

diff --git a/offload/liboffload/src/OffloadImpl.cpp b/offload/liboffload/src/OffloadImpl.cpp
index 27c57232b..cb5ae8e19 100644
--- a/offload/liboffload/src/OffloadImpl.cpp
+++ b/offload/liboffload/src/OffloadImpl.cpp
@@ -1232,7 +1232,7 @@ Error olLaunchHostFunction_impl(ol_queue_handle_t Queue,
 Error olDataFence_impl(ol_queue_handle_t Queue) {
   if (Queue->AsyncInfo->Queue) {
     if (auto Err = Queue->Device->Device->dataFence(Queue->AsyncInfo))
-      return Err; 
+      return Err;
   }
 
   return Error::success();
@@ -1241,13 +1241,14 @@ Error olDataFence_impl(ol_queue_handle_t Queue) {
 Error olQueryAsync_impl(ol_queue_handle_t Queue) {
   if (Queue->AsyncInfo->Queue) {
     if (auto Err = Queue->Device->Device->queryAsync(Queue->AsyncInfo))
-      return Err; 
+      return Err;
   }
 
   return Error::success();
 }
 
-Error olMemDataMappedNotify_impl(ol_device_handle_t Device, void *Ptr, size_t Size) {
+Error olMemDataMappedNotify_impl(ol_device_handle_t Device, void *Ptr,
+                                 size_t Size) {
   return Device->Device->notifyDataMapped(Ptr, Size);
 }
 
@@ -1255,8 +1256,9 @@ Error olMemDataUnMappedNotify_impl(ol_device_handle_t Device, void *Ptr) {
   return Device->Device->notifyDataUnmapped(Ptr);
 }
 
-Error olMemDataLock_impl(ol_device_handle_t Device, void *Ptr, size_t Size, void** LockedPtr) {
-  Expected<void*> LockedPtrOrErr = Device->Device->dataLock(Ptr, Size);
+Error olMemDataLock_impl(ol_device_handle_t Device, void *Ptr, size_t Size,
+                         void **LockedPtr) {
+  Expected<void *> LockedPtrOrErr = Device->Device->dataLock(Ptr, Size);
   if (!LockedPtrOrErr)
     return LockedPtrOrErr.takeError();
 
@@ -1269,32 +1271,37 @@ Error olMemDataUnLock_impl(ol_device_handle_t Device, void *Ptr) {
   return Device->Device->dataUnlock(Ptr);
 }
 
-Error olCreateInterop_impl(ol_device_handle_t Device, int32_t InteropContext, void* InteropSpec, void **Interop) {
-  auto Rc = Device->Device->createInterop(InteropContext, *(interop_spec_t *)InteropSpec);
+Error olCreateInterop_impl(ol_device_handle_t Device, int32_t InteropContext,
+                           void *InteropSpec, void **Interop) {
+  auto Rc = Device->Device->createInterop(InteropContext,
+                                          *(interop_spec_t *)InteropSpec);
   if (!Rc)
     return Rc.takeError();
   *Interop = *Rc;
   return Error::success();
 }
 
-Error olReleaseInterop_impl(ol_device_handle_t Device, void* InteropSpec) {
+Error olReleaseInterop_impl(ol_device_handle_t Device, void *InteropSpec) {
   auto Rc = Device->Device->releaseInterop((omp_interop_val_t *)InteropSpec);
   if (Rc)
     return Rc;
   return Error::success();
 }
 
-Error olSelectInterop_impl(ol_device_handle_t Device, int32_t InteropType, int32_t InteropPreferencesSize, 
-        void *InteropPreferences, void* InteropSpec) {
-  *(interop_spec_t*)InteropSpec = Device->Device->selectInteropPreference(
-      InteropType, InteropPreferencesSize, (interop_spec_t *)InteropPreferences);
+Error olSelectInterop_impl(ol_device_handle_t Device, int32_t InteropType,
+                           int32_t InteropPreferencesSize,
+                           void *InteropPreferences, void *InteropSpec) {
+  *(interop_spec_t *)InteropSpec = Device->Device->selectInteropPreference(
+      InteropType, InteropPreferencesSize,
+      (interop_spec_t *)InteropPreferences);
   return Error::success();
 }
 
 Error olFlushQueueInterop_impl(ol_device_handle_t Device, void *Interop) {
 
-  Expected<int32_t> Rc = Device->Device->Plugin.flush_queue((omp_interop_val_t *)Interop);
-  if (Rc){
+  Expected<int32_t> Rc =
+      Device->Device->Plugin.flush_queue((omp_interop_val_t *)Interop);
+  if (Rc) {
     return Rc.takeError();
   }
   return Error::success();
@@ -1302,8 +1309,9 @@ Error olFlushQueueInterop_impl(ol_device_handle_t Device, void *Interop) {
 
 Error olSyncBarrierInterop_impl(ol_device_handle_t Device, void *Interop) {
 
-  Expected<int32_t> Rc = Device->Device->Plugin.sync_barrier((omp_interop_val_t *)Interop);
-  if (Rc){
+  Expected<int32_t> Rc =
+      Device->Device->Plugin.sync_barrier((omp_interop_val_t *)Interop);
+  if (Rc) {
     return Rc.takeError();
   }
   return Error::success();
@@ -1311,22 +1319,23 @@ Error olSyncBarrierInterop_impl(ol_device_handle_t Device, void *Interop) {
 
 Error olAsyncBarrierInterop_impl(ol_device_handle_t Device, void *Interop) {
 
-  Expected<int32_t> Rc = Device->Device->Plugin.async_barrier((omp_interop_val_t *)Interop);
-  if (Rc){
+  Expected<int32_t> Rc =
+      Device->Device->Plugin.async_barrier((omp_interop_val_t *)Interop);
+  if (Rc) {
     return Rc.takeError();
   }
   return Error::success();
 }
 
-Error olInitializeRecordReplay_impl(ol_device_handle_t Device, uint64_t MemorySize,
-                                        void *VAddr, bool IsRecord,
-                                        bool SaveOutput,
-                                        uint64_t *ReqPtrArgOffset){
+Error olInitializeRecordReplay_impl(ol_device_handle_t Device,
+                                    uint64_t MemorySize, void *VAddr,
+                                    bool IsRecord, bool SaveOutput,
+                                    uint64_t *ReqPtrArgOffset) {
   uint64_t ReqPtrArgOffsetOut = 0;
-  Expected<int> Rc = Device->Device->Plugin.initialize_record_replay(Device->DeviceNum, MemorySize,
-                                              VAddr, IsRecord, SaveOutput,
-                                              ReqPtrArgOffsetOut);
-  if (Rc){
+  Expected<int> Rc = Device->Device->Plugin.initialize_record_replay(
+      Device->DeviceNum, MemorySize, VAddr, IsRecord, SaveOutput,
+      ReqPtrArgOffsetOut);
+  if (Rc) {
     return Rc.takeError();
   }
   *ReqPtrArgOffset = ReqPtrArgOffsetOut;

adurang · 2025-12-09T16:32:33Z

As a general comment, divide this into multiple PRs (only put together APIs that are related).

fineg74 · 2025-12-14T22:11:26Z

Split the PR to :
#172222
#172223
#172226
#172229
#172231
#172232

Add missing API for libomptarget migration

6b718b2

Merge branch 'main' into libOffloadAPI

e3a1b24

sarnex reviewed Dec 8, 2025

View reviewed changes

Split the PR per comments

c183319

fineg74 changed the title ~~[OFFLOAD] Add missing API for libomptarget migration~~ [OFFLOAD] Add missing platform properties for libomptarget migration Dec 14, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[OFFLOAD] Add missing platform properties for libomptarget migration #171009

[OFFLOAD] Add missing platform properties for libomptarget migration #171009

fineg74 commented Dec 7, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Dec 7, 2025

Uh oh!

sarnex left a comment

Uh oh!

sarnex Dec 8, 2025

Uh oh!

sarnex Dec 8, 2025

Uh oh!

sarnex Dec 8, 2025

Uh oh!

sarnex Dec 8, 2025

Uh oh!

fineg74 Dec 9, 2025

Uh oh!

sarnex Dec 8, 2025

Uh oh!

github-actions bot commented Dec 8, 2025

Uh oh!

adurang commented Dec 9, 2025

Uh oh!

fineg74 commented Dec 14, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[OFFLOAD] Add missing platform properties for libomptarget migration #171009

Are you sure you want to change the base?

[OFFLOAD] Add missing platform properties for libomptarget migration #171009

Conversation

fineg74 commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 7, 2025

Uh oh!

sarnex left a comment

Choose a reason for hiding this comment

Uh oh!

sarnex Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

sarnex Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

sarnex Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

sarnex Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

fineg74 Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

sarnex Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 8, 2025

Uh oh!

adurang commented Dec 9, 2025

Uh oh!

fineg74 commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fineg74 commented Dec 7, 2025 •

edited

Loading

fineg74 commented Dec 14, 2025 •

edited

Loading