[OpenMP] Add placeholder functions for the depend and nowait depend clauses for target data directives.

[gtbercea]

Summary

This patch adds placeholder functions to the interface for depend and nowait depend OpenMP target data begin, end and update directives.

[Hahnfeld]

These functions are 1:1 the same as the current _nowait functions. If that's for handling depend clauses without nowait I think the compiler can just add another runtime call into libomp.

[grokos]

In D50158#1185361, @Hahnfeld wrote:

These functions are 1:1 the same as the current _nowait functions. If that's for handling depend clauses without nowait I think the compiler can just add another runtime call into libomp.

Is there any benefit in having the compiler generate the call to libomp vs. embedding this call in extra interface functions?

[Hahnfeld]

In D50158#1185859, @grokos wrote:

In D50158#1185361, @Hahnfeld wrote:

These functions are 1:1 the same as the current _nowait functions. If that's for handling depend clauses without nowait I think the compiler can just add another runtime call into libomp.

Is there any benefit in having the compiler generate the call to libomp vs. embedding this call in extra interface functions?

Is there any benefit of having more functions? The interface of libomp is already there...

[RaviNarayanaswamy]

We should not be having calls to _-kmpc_omp_taskwait in libomptarget.
The wait should be done in the user code and then libomptarget routine should be called.

[Hahnfeld]

In D50158#1185906, @RaviNarayanaswamy wrote:

We should not be having calls to _-kmpc_omp_taskwait in libomptarget.
The wait should be done in the user code and then libomptarget routine should be called.

There is no way to implement asynchronous offloading without having knowledge of the dependencies. Intel failed to keep the separation strict for liboffload, so I don't think it helps to complain about the situation.

[ABataev]

In D50158#1185919, @Hahnfeld wrote:

In D50158#1185906, @RaviNarayanaswamy wrote:

We should not be having calls to _-kmpc_omp_taskwait in libomptarget.
The wait should be done in the user code and then libomptarget routine should be called.

There is no way to implement asynchronous offloading without having knowledge of the dependencies. Intel failed to keep the separation strict for liboffload, so I don't think it helps to complain about the situation.

Instead, we may pass the pointer to the synchronization function and call it. The compiler may provide the pointer to this sync function. The expected function may be simple (like void sync()), the compiler may generate the required wrapper function around __kmpc_omp_taskwait

[Hahnfeld]

In D50158#1185936, @ABataev wrote:

In D50158#1185919, @Hahnfeld wrote:

In D50158#1185906, @RaviNarayanaswamy wrote:

We should not be having calls to _-kmpc_omp_taskwait in libomptarget.
The wait should be done in the user code and then libomptarget routine should be called.

There is no way to implement asynchronous offloading without having knowledge of the dependencies. Intel failed to keep the separation strict for liboffload, so I don't think it helps to complain about the situation.

Instead, we may pass the pointer to the synchronization function and call it. The compiler may provide the pointer to this sync function. The expected function may be simple (like void sync()), the compiler may generate the required wrapper function around __kmpc_omp_taskwait

When truly implementing asynchronous offloading libomptarget needs to tightly integrate into libomp's task scheduling. This will require a larger set of functions and I don't think we should change the interface just because we want to improve the implementation.

Anyway it's libOMPtarget, I don't really see the point. The library isn't agnostic of the programming model, it obeys to OMP_ environment variables - which btw. are also queried from libomp!

[RaviNarayanaswamy]

It may be named libOMPtarget and is in openmp projects but we want to use the offloading library with other runtime libraries.

[gtbercea]

These calls are here because they require the interface of libomp library include these functions. A patch for Clang is in the works which calls these functions so they need to have some basic, correct implementation that works when used with libomp.
The implementation can/should be improved in the future. In our proprietary OpenMP library implementation we already do something more elaborate which is why we need the placeholders here.

[gtbercea]

! In D50158#1185865, @Hahnfeld wrote:
Is there any benefit of having more functions? The interface of libomp is already there...

Yes, libomptarget can be used with other versions of the libomp library which may have a richer interface than the open source openmp runtime library.

[Hahnfeld]

In D50158#1185974, @gtbercea wrote:

These calls are here because they require the interface of libomp library include these functions. A patch for Clang is in the works which calls these functions so they need to have some basic, correct implementation that works when used with libomp.
The implementation can/should be improved in the future. In our proprietary OpenMP library implementation we already do something more elaborate which is why we need the placeholders here.

Can you at least partly explain why the compiler generated code cannot call __kmpc_omp_taskwait for directives that have no nowait clause? Including a change because "we need it for our proprietary implementation" is a weak argument IMO...

[AlexEichenberger]

Implementation:

First, there missing sync for the _tgt_target_data_begin_depend, tgt_target_data_begin_depend_nowait, tgt_target_data_update_depend, tgt_target_data_update_depend_nowait,
Look at the end versions of the call for a possible implementation.
Second, it would be better IMO to use kmpc_omp_wait_deps instead of the broader task wait.

Larger picture:

We negotiated these calls with Alexey: we could create target tasks for these, but Alexey estimated that creating target tasks for entries like begin/end/update, which have no target code, was significantly more complicated. And since it's not a big deal to do it in the runtime, that is why we agreed on having the async versions of these as entry points. If Alexey want to revisit his judgement, I have no issue with that. Its a lot of busy work for something that can easily be replaced in the runtime, but fine by me either ways.

Bigger pictures:

In D50158#1185943, @Hahnfeld wrote:

When truly implementing asynchronous offloading libomptarget needs to tightly integrate into libomp's task scheduling. This will require a larger set of functions and I don't think we should change the interface just because we want to improve the implementation.

Anyway it's libOMPtarget, I don't really see the point. The library isn't agnostic of the programming model, it obeys to OMP_ environment variables - which btw. are also queried from libomp!

Agreed, and what is needed here is a way to launch a target task, have libOMP filter dependences that can be satisfied on the device and the one that cannot. The ones that can be satisfied on the device, we need an token that will in effect describe an event/stream combination for NVIDIA, and they will be considered as satisfied from the host's perspective. The ones that are not satisfied on the device, the hose will have to wait for these as usual. Then once all the host dependences are satisfied, the actual task can be launched with the device dependence (list of token to event/stream) being inserted by libomptarget.

The mechanism in libOMP is fairly unobtrusive, in that the dependence graph nodes need to be augmented by one device descriptor (host or device id) and one dependence descriptor (which is device dependent). When scanning a target task, the compiler has to create a task with extra data to maintain up to one pointer per dependence. So the filter add the dependences that can be satisfied by the device in that extra storage associated with the task. When the target task is ready to launch (i.e. all the host dependences are satisfied), libomptarget will get this list of filtered dependences and act accordingly.

[Hahnfeld]

In D50158#1186058, @AlexEichenberger wrote:

Implementation:

First, there missing sync for the _tgt_target_data_begin_depend, tgt_target_data_begin_depend_nowait, tgt_target_data_update_depend, __tgt_target_data_update_depend_nowait,
Look at the end versions of the call for a possible implementation.

I'm not sure what you mean here: libomptarget is completely synchronous for now, so the internal calls will only return after all data movement has happened. Or am I missing something here?

Second, it would be better IMO to use __kmpc_omp_wait_deps instead of the broader task wait.

Agreed. Didn't we have that at some point?

Larger picture:

We negotiated these calls with Alexey: we could create target tasks for these, but Alexey estimated that creating target tasks for entries like begin/end/update, which have no target code, was significantly more complicated. And since it's not a big deal to do it in the runtime, that is why we agreed on having the async versions of these as entry points. If Alexey want to revisit his judgement, I have no issue with that. Its a lot of busy work for something that can easily be replaced in the runtime, but fine by me either ways.

I agree with both of you here: It's easier to do in the runtime + extra optimization opportunities.

Bigger pictures:

In D50158#1185943, @Hahnfeld wrote:

When truly implementing asynchronous offloading libomptarget needs to tightly integrate into libomp's task scheduling. This will require a larger set of functions and I don't think we should change the interface just because we want to improve the implementation.

Anyway it's libOMPtarget, I don't really see the point. The library isn't agnostic of the programming model, it obeys to OMP_ environment variables - which btw. are also queried from libomp!

Agreed, and what is needed here is a way to launch a target task, have libOMP filter dependences that can be satisfied on the device and the one that cannot. The ones that can be satisfied on the device, we need an token that will in effect describe an event/stream combination for NVIDIA, and they will be considered as satisfied from the host's perspective. The ones that are not satisfied on the device, the hose will have to wait for these as usual. Then once all the host dependences are satisfied, the actual task can be launched with the device dependence (list of token to event/stream) being inserted by libomptarget.

That's actually the point why you need a dedicated function for depend without nowait. I still don't see why there is a need for _nowait_depend, all of that is already handled by _nowait with an empty dependency list.

[gtbercea]

No longer needed for trunk.

[AlexEichenberger]

agreed

[Hahnfeld]

Hmm, as far as I understood there is a case for needing _depend (without nowait)

libomptarget/src/exports

 VERS1.0 {
   global:
     __tgt_register_lib;
     __tgt_unregister_lib;
     __tgt_target_data_begin;
     __tgt_target_data_end;
     __tgt_target_data_update;
     __tgt_target;
     __tgt_target_teams;
     __tgt_target_data_begin_nowait;
     __tgt_target_data_end_nowait;
     __tgt_target_data_update_nowait;
     __tgt_target_nowait;
     __tgt_target_teams_nowait;
+    __tgt_target_data_begin_depend;
+    __tgt_target_data_end_depend;
+    __tgt_target_data_update_depend;
+    __tgt_target_data_begin_nowait_depend;
+    __tgt_target_data_end_nowait_depend;
+    __tgt_target_data_update_nowait_depend;
     omp_get_num_devices;
     omp_get_initial_device;
     omp_target_alloc;
     omp_target_free;
     omp_target_is_present;
     omp_target_memcpy;
     omp_target_memcpy_rect;
     omp_target_associate_ptr;
     omp_target_disassociate_ptr;
     __kmpc_push_target_tripcount;
   local:
     *;
 };
 

libomptarget/src/interface.cpp

EXTERN void __tgt_target_data_begin_nowait(int64_t device_id, int32_t arg_num,
     void *noAliasDepList) {
   if (depNum + noAliasDepNum > 0)
     __kmpc_omp_taskwait(NULL, 0);
 
   __tgt_target_data_begin(device_id, arg_num, args_base, args, arg_sizes,
                           arg_types);
 }
 
+EXTERN void __tgt_target_data_begin_depend(int64_t device_id, int32_t arg_num,
+    void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types,
+    int32_t depNum, void *depList, int32_t noAliasDepNum,
+    void *noAliasDepList) {
+  if (depNum + noAliasDepNum > 0)
+    __kmpc_omp_taskwait(NULL, 0);
+
+  __tgt_target_data_begin(device_id, arg_num, args_base, args, arg_sizes,
+                          arg_types);
+}
+
+EXTERN void __tgt_target_data_begin_nowait_depend(int64_t device_id,
+    int32_t arg_num, void **args_base, void **args, int64_t *arg_sizes,
+    int64_t *arg_types, int32_t depNum, void *depList, int32_t noAliasDepNum,
+    void *noAliasDepList) {
+  if (depNum + noAliasDepNum > 0)
+    __kmpc_omp_taskwait(NULL, 0);
+
+  // Async not yet implemented, just call the blocking version
+  __tgt_target_data_begin(device_id, arg_num, args_base, args, arg_sizes,
+                          arg_types);
+}
+
 /// 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) {
   DP("Entering data end region with %d mappings\n", arg_num);
 
   // No devices available?
EXTERN void __tgt_target_data_end_nowait(int64_t device_id, int32_t arg_num,
     void *noAliasDepList) {
   if (depNum + noAliasDepNum > 0)
     __kmpc_omp_taskwait(NULL, 0);
 
   __tgt_target_data_end(device_id, arg_num, args_base, args, arg_sizes,
                         arg_types);
 }
 
+EXTERN void __tgt_target_data_end_depend(int64_t device_id, int32_t arg_num,
+    void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types,
+    int32_t depNum, void *depList, int32_t noAliasDepNum,
+    void *noAliasDepList) {
+  if (depNum + noAliasDepNum > 0)
+    __kmpc_omp_taskwait(NULL, 0);
+
+  __tgt_target_data_end(device_id, arg_num, args_base, args, arg_sizes,
+                          arg_types);
+}
+
+EXTERN void __tgt_target_data_end_nowait_depend(int64_t device_id,
+    int32_t arg_num, void **args_base, void **args, int64_t *arg_sizes,
+    int64_t *arg_types, int32_t depNum, void *depList, int32_t noAliasDepNum,
+    void *noAliasDepList) {
+  if (depNum + noAliasDepNum > 0)
+    __kmpc_omp_taskwait(NULL, 0);
+
+  // Async not yet implemented, just call the blocking version
+  __tgt_target_data_end(device_id, arg_num, args_base, args, arg_sizes,
+                          arg_types);
+}
+
 EXTERN void __tgt_target_data_update(int64_t device_id, int32_t arg_num,
     void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types) {
   DP("Entering data update with %d mappings\n", arg_num);
 
   // No devices available?
   if (device_id == OFFLOAD_DEVICE_DEFAULT) {
     device_id = omp_get_default_device();
   }
EXTERN void __tgt_target_data_update_nowait(
     int32_t noAliasDepNum, void *noAliasDepList) {
   if (depNum + noAliasDepNum > 0)
     __kmpc_omp_taskwait(NULL, 0);
 
   __tgt_target_data_update(device_id, arg_num, args_base, args, arg_sizes,
                            arg_types);
 }
 
+EXTERN void __tgt_target_data_update_depend(int64_t device_id, int32_t arg_num,
+    void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types,
+    int32_t depNum, void *depList, int32_t noAliasDepNum,
+    void *noAliasDepList) {
+  if (depNum + noAliasDepNum > 0)
+    __kmpc_omp_taskwait(NULL, 0);
+
+  __tgt_target_data_update(device_id, arg_num, args_base, args, arg_sizes,
+                          arg_types);
+}
+
+EXTERN void __tgt_target_data_update_nowait_depend(int64_t device_id,
+    int32_t arg_num, void **args_base, void **args, int64_t *arg_sizes,
+    int64_t *arg_types, int32_t depNum, void *depList, int32_t noAliasDepNum,
+    void *noAliasDepList) {
+  if (depNum + noAliasDepNum > 0)
+    __kmpc_omp_taskwait(NULL, 0);
+
+  // Async not yet implemented, just call the blocking version
+  __tgt_target_data_update(device_id, arg_num, args_base, args, arg_sizes,
+                          arg_types);
+}
+
 EXTERN int __tgt_target(int64_t device_id, void *host_ptr, int32_t arg_num,
     void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types) {
   DP("Entering target region with entry point " DPxMOD " and device Id %"
       PRId64 "\n", DPxPTR(host_ptr), device_id);
 
   if (device_id == OFFLOAD_DEVICE_DEFAULT) {
     device_id = omp_get_default_device();
   }