[OpenMPOpt][SplitMemTransfer] Implementation of the real issue and wait api functions.

[hamax97]

Summary

Implementations of the issue and wait version of __tgt_target_data_begin_mapper.

[hamax97]

Passing handle as input/output argument to __tgt_target_data_begin_mapper_<issue|wait>.

• The diff ended up weird, I just added the declarations and definitions for __tgt_target_data_begin_mapper_issue and __tgt_target_data_begin_mapper_wait. I didn't deleted any code.

[hamax97]

Ping!. Could you please take a look at it?

[grokos]

I think these API functions should also include the source location pointer from https://reviews.llvm.org/D87946. We need to consider renaming the *_issue and *_wait functions to extend the *_loc API the aforementioned patch is introducing. E.g. after D87946 the "current" data begin API function will be __tgt_target_data_begin_mapper_loc, so this patch should extend that name as __tgt_target_data_begin_mapper_loc_issue and __tgt_target_data_begin_mapper_loc_wait. Because both patches make changes to the API, I think it's better to wait until the former patch has been committed.

[jhuber6]

In D87259#2301065, @grokos wrote:

I think these API functions should also include the source location pointer from https://reviews.llvm.org/D87946. We need to consider renaming the *_issue and *_wait functions to extend the *_loc API the aforementioned patch is introducing. E.g. after D87946 the "current" data begin API function will be __tgt_target_data_begin_mapper_loc, so this patch should extend that name as __tgt_target_data_begin_mapper_loc_issue and __tgt_target_data_begin_mapper_loc_wait. Because both patches make changes to the API, I think it's better to wait until the former patch has been committed.

We've also been talking about extending the API to optionally include declaration names in the mappers so we can refer to mapped variables by name in error and information messages. If we're breaking the API it's probably best to do it all at once so I'm not sure how soon we'll be able to push D87946.

[hamax97]

In D87259#2301065, @grokos wrote:

I think these API functions should also include the source location pointer from https://reviews.llvm.org/D87946. We need to consider renaming the *_issue and *_wait functions to extend the *_loc API the aforementioned patch is introducing. E.g. after D87946 the "current" data begin API function will be __tgt_target_data_begin_mapper_loc, so this patch should extend that name as __tgt_target_data_begin_mapper_loc_issue and __tgt_target_data_begin_mapper_loc_wait. Because both patches make changes to the API, I think it's better to wait until the former patch has been committed.

Yes. It will be easier to add this patch after D87946 and not the other way around.

openmp/libomptarget/include/omptarget.h

 void __tgt_target_data_begin_mapper(int64_t device_id, int32_t arg_num,
                                     void **args_base, void **args,
                                     int64_t *arg_sizes, int64_t *arg_types,
                                     void **arg_mappers);
 void __tgt_target_data_begin_nowait_mapper(
     int64_t device_id, int32_t arg_num, void **args_base, void **args,
     int64_t *arg_sizes, int64_t *arg_types, void **arg_mappers, int32_t depNum,
     void *depList, int32_t noAliasDepNum, void *noAliasDepList);
+__tgt_async_info *__tgt_target_data_begin_mapper_issue(
+    int64_t device_id, int32_t arg_num, void **args_base, void **args,
+    int64_t *arg_sizes, int64_t *arg_types, void **arg_mappers);
+void __tgt_target_data_begin_mapper_wait(int32_t device_id,
+                                         __tgt_async_info *handle);
 
 // passes data from the target, release target memory and destroys the
 // host-target mapping (top entry from the stack of data maps) created by
 // the last __tgt_target_data_begin
 void __tgt_target_data_end(int64_t device_id, int32_t arg_num, void **args_base,
                            void **args, int64_t *arg_sizes, int64_t *arg_types);
 void __tgt_target_data_end_nowait(int64_t device_id, int32_t arg_num,
                                   void **args_base, void **args,

openmp/libomptarget/src/interface.cpp

EXTERN void __tgt_target_data_begin_nowait(int64_t device_id, int32_t arg_num,
 
   __tgt_target_data_begin_mapper(device_id, arg_num, args_base, args,
       arg_sizes, arg_types, nullptr);
 }
 
 EXTERN void __tgt_target_data_begin_mapper(int64_t device_id, int32_t arg_num,
     void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types,
     void **arg_mappers) {
-  if (IsOffloadDisabled()) return;
+
+  __tgt_async_info *Handle =
+      __tgt_target_data_begin_mapper_issue(device_id,arg_num, args_base, args,
+                                           arg_sizes, arg_types, arg_mappers);
+  __tgt_target_data_begin_mapper_wait(device_id, Handle);
+}
+
+EXTERN void __tgt_target_data_begin_nowait_mapper(int64_t device_id,
+    int32_t arg_num, void **args_base, void **args, int64_t *arg_sizes,
+    int64_t *arg_types, void **arg_mappers, int32_t depNum, void *depList,
+    int32_t noAliasDepNum, void *noAliasDepList) {
+  if (depNum + noAliasDepNum > 0)
+    __kmpc_omp_taskwait(NULL, __kmpc_global_thread_num(NULL));
+
+  __tgt_target_data_begin_mapper(device_id, arg_num, args_base, args,
+      arg_sizes, arg_types, arg_mappers);
+}
+
+EXTERN __tgt_async_info *__tgt_target_data_begin_mapper_issue(
+    int64_t device_id, int32_t arg_num, void **args_base, void **args,
+    int64_t *arg_sizes, int64_t *arg_types, void **arg_mappers) {
+
+  __tgt_async_info *Handle =
+      (__tgt_async_info *) malloc(sizeof(__tgt_async_info));
+  if (IsOffloadDisabled()) return Handle;
 
   DP("Entering data begin region for device %" PRId64 " with %d mappings\n",
-      device_id, arg_num);
 
   // No devices available?
   if (device_id == OFFLOAD_DEVICE_DEFAULT) {
     device_id = omp_get_default_device();
     DP("Use default device id %" PRId64 "\n", device_id);
   }
 
   if (CheckDeviceAndCtors(device_id) != OFFLOAD_SUCCESS) {
     DP("Failed to get device %" PRId64 " ready\n", device_id);
     HandleTargetOutcome(false);
-    return;
+    return Handle;
   }
 
   DeviceTy &Device = Devices[device_id];
 
 #ifdef OMPTARGET_DEBUG
   for (int i = 0; i < arg_num; ++i) {
     DP("Entry %2d: Base=" DPxMOD ", Begin=" DPxMOD ", Size=%" PRId64
            ", Type=0x%" PRIx64 "\n",
        i, DPxPTR(args_base[i]), DPxPTR(args[i]), arg_sizes[i], arg_types[i]);
   }
 #endif
 
   int rc = targetDataBegin(Device, arg_num, args_base, args, arg_sizes,
-                           arg_types, arg_mappers, nullptr);
+                           arg_types, arg_mappers, Handle);
   HandleTargetOutcome(rc == OFFLOAD_SUCCESS);
+
+  return Handle;
 }
 
-EXTERN void __tgt_target_data_begin_nowait_mapper(int64_t device_id,
-    int32_t arg_num, void **args_base, void **args, int64_t *arg_sizes,
-    int64_t *arg_types, void **arg_mappers, int32_t depNum, void *depList,
-    int32_t noAliasDepNum, void *noAliasDepList) {
-  if (depNum + noAliasDepNum > 0)
-    __kmpc_omp_taskwait(NULL, __kmpc_global_thread_num(NULL));
+EXTERN void __tgt_target_data_begin_mapper_wait(int32_t device_id,
+                                                __tgt_async_info *handle) {
+  if (!handle || !handle->Queue) {
+    DP("No handle to wait on!");
+    HandleTargetOutcome(false);
+    return;
+  }
 
-  __tgt_target_data_begin_mapper(device_id, arg_num, args_base, args,
-      arg_sizes, arg_types, arg_mappers);
+  // No devices available?
+  if (device_id == OFFLOAD_DEVICE_DEFAULT)
+    device_id = omp_get_default_device();
+
+  DeviceTy &Device = Devices[device_id];
+  Device.synchronize(handle);
 }
 
 /// passes data from the target, releases target memory and destroys
 /// the host-target mapping (top entry from the stack of data maps)
 /// created by the last __tgt_target_data_begin.
 EXTERN void __tgt_target_data_end(int64_t device_id, int32_t arg_num,
     void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types) {
   __tgt_target_data_end_mapper(device_id, arg_num, args_base, args, arg_sizes,