[Openmp-commits] [PATCH] D50158: [OpenMP] Add placeholder functions for the depend and nowait depend clauses for target data directives.

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/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 https://reviews.llvm.org/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.


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D50158