[Openmp-commits] [PATCH] D96379: [OpenMP] Unify omptarget API and usage wrt. `__tgt_async_info`

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added inline comments.


================
Comment at: openmp/libomptarget/src/interface.cpp:485
 
-  int rc = target(loc, device_id, host_ptr, arg_num, args_base, args, arg_sizes,
+  DeviceTy &Device = PM->Devices[device_id];
+  __tgt_async_info AsyncInfo;
----------------
It's good to unify the usage of device id and device object. Probably better to separate it to another patch.


================
Comment at: openmp/libomptarget/src/interface.cpp:487
+  __tgt_async_info AsyncInfo;
+  int rc = target(loc, Device, host_ptr, arg_num, args_base, args, arg_sizes,
                   arg_types, arg_names, arg_mappers, team_num, thread_limit,
----------------
It's really "uncomfortable" to read the code mixed with different styles, like `rc` and `AsyncInfo`.


================
Comment at: openmp/libomptarget/src/omptarget.cpp:1318
     // variables
-    Ret = processDataAfter(loc, DeviceId, HostPtr, ArgNum, ArgBases, Args,
-                           ArgSizes, ArgTypes, ArgNames, ArgMappers,
-                           PrivateArgumentManager, &AsyncInfo);
+    Ret = processDataAfter(loc, Device.DeviceID, HostPtr, ArgNum, ArgBases,
+                           Args, ArgSizes, ArgTypes, ArgNames, ArgMappers,
----------------
We might want a unified use of either device id or device object.


================
Comment at: openmp/libomptarget/src/private.h:16
 
+#include "rtl.h"
 #include <Debug.h>
----------------
Why do we need this include?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96379/new/

https://reviews.llvm.org/D96379