[gomp4.1] depend nowait support for target {update,{enter,exit} data}

On Thu, Oct 15, 2015 at 16:01:56 +0200, Jakub Jelinek wrote:
> >    void *fn_addr = gomp_get_target_fn_addr (devicep, fn);
> >  
> > +  if (flags & GOMP_TARGET_FLAG_NOWAIT)
> > +    {
> > +      gomp_create_target_task (devicep, fn_addr, mapnum, hostaddrs, sizes,
> > +			       kinds, flags, depend);
> > +      return;
> > +    }
> 
> But this is not ok.  You need to do this far earlier, already before the
> if (depend != NULL) code in GOMP_target_41.  And, I think you should just
> not pass fn_addr, but fn itself.
> 
> > @@ -1636,34 +1657,58 @@ void
> >  gomp_target_task_fn (void *data)
> >  {
> >    struct gomp_target_task *ttask = (struct gomp_target_task *) data;
> > +  struct gomp_device_descr *devicep = ttask->devicep;
> > +
> >    if (ttask->fn != NULL)
> >      {
> > -      /* GOMP_target_41 */
> > +      if (devicep == NULL
> > +	  || !(devicep->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400))
> > +	{
> > +	  /* FIXME: Save host fn addr into gomp_target_task?  */
> > +	  gomp_target_fallback_firstprivate (NULL, ttask->mapnum,
> 
> If you pass above fn instead of fn_addr, ttask->fn is what you want
> to pass to gomp_target_fallback_firstprivate here and remove the FIXME.
> 
> > +					     ttask->hostaddrs, ttask->sizes,
> > +					     ttask->kinds);
> > +	  return;
> > +	}
> > +
> > +      struct target_mem_desc *tgt_vars
> > +	= gomp_map_vars (devicep, ttask->mapnum, ttask->hostaddrs, NULL,
> > +			 ttask->sizes, ttask->kinds, true,
> > +			 GOMP_MAP_VARS_TARGET);
> > +      devicep->async_run_func (devicep->target_id, ttask->fn,
> > +			       (void *) tgt_vars->tgt_start, data);
> 
> You need to void *fn_addr = gomp_get_target_fn_addr (devicep, ttask->fn);
> first obviously, and pass fn_addr.
> 
> > +
> > +      /* FIXME: TMP example of checking for completion.
> > +	 Alternatively the plugin can set some completion flag in ttask.  */
> > +      while (!devicep->async_is_completed_func (devicep->target_id, data))
> > +	{
> > +	  fprintf (stderr, "-");
> > +	  usleep (100000);
> > +	}
> 
> This obviously doesn't belong here.
> 
> >    if (device->capabilities & GOMP_OFFLOAD_CAP_OPENACC_200)
> > diff --git a/libgomp/testsuite/libgomp.c/target-tmp.c b/libgomp/testsuite/libgomp.c/target-tmp.c
> > new file mode 100644
> > index 0000000..23a739c
> > --- /dev/null
> > +++ b/libgomp/testsuite/libgomp.c/target-tmp.c
> > @@ -0,0 +1,40 @@
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +
> > +#pragma omp declare target
> > +void foo (int n)
> > +{
> > +  printf ("Start tgt %d\n", n);
> > +  usleep (5000000);
> 
> 5s is too long.  Not to mention that not sure if PTX can do printf
> and especially usleep.
> 
> > diff --git a/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp b/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
> > index 26ac6fe..c843710 100644
> > --- a/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
> > +++ b/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
> ...
> > +/* Set of asynchronously running target tasks.  */
> > +static std::set<const void *> *async_tasks;
> > +
> >  /* Thread-safe registration of the main image.  */
> >  static pthread_once_t main_image_is_registered = PTHREAD_ONCE_INIT;
> >  
> > +/* Mutex for protecting async_tasks.  */
> > +static pthread_mutex_t async_tasks_lock = PTHREAD_MUTEX_INITIALIZER;
> > +
> >  static VarDesc vd_host2tgt = {
> >    { 1, 1 },		      /* dst, src			      */
> >    { 1, 0 },		      /* in, out			      */
> > @@ -156,6 +163,8 @@ init (void)
> >  
> >  out:
> >    address_table = new ImgDevAddrMap;
> > +  async_tasks = new std::set<const void *>;
> > +  pthread_mutex_init (&async_tasks_lock, NULL);
> 
> PTHREAD_MUTEX_INITIALIZER should already initialize the lock.
> But, do you really need async_tasks and the lock?  Better store
> something into some plugin's owned field in target_task struct and
> let the plugin callback be passed address of that field rather than the
> whole target_task?

So, here is what I have for now.  Attached target-29.c testcase works fine with
MIC emul, however I don't know how to (and where) properly check for completion
of async execution on target.  And, similarly, where to do unmapping after that?
Do we need a callback from plugin to libgomp (as far as I understood, PTX
runtime supports this, but HSA doesn't), or libgomp will just check for
ttask->is_completed in task.c?

Thanks,
  -- Ilya

[gomp4.1] depend nowait support for target {update,{enter,exit} data}

Commit Message

Patch