[Bug libgomp/97212] New: [OpenMP] 'depend' clause with 'target nowait' (!) + 'task' does not work

[Bug libgomp/97212] New: [OpenMP] 'depend' clause with 'target nowait' (!) + 'task' does not work

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97212

            Bug ID: 97212
           Summary: [OpenMP] 'depend' clause with 'target nowait' (!) +
                    'task' does not work
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: openmp, wrong-code
          Severity: normal
          Priority: P3
         Component: libgomp
          Assignee: unassigned at gcc dot gnu.org
          Reporter: burnus at gcc dot gnu.org
                CC: jakub at gcc dot gnu.org
  Target Milestone: ---

Created attachment 49274
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49274&action=edit
C testcase, run with -fopenmp

The SOLLVE_VV testcase
https://github.com/SOLLVE/sollve_vv/blob/master/tests/4.5/task/test_target_and_task_nowait.c

FAILS. Note: It also fails with a compiler which is not even configured for
offloading and, hence, everything is run on the host.


It uses with 'nowait' and 'depend':

#pragma omp target map(tofrom: a, sum) depend(out: a) nowait
  ... (set 'a') ...

#pragma omp task depend(in: a) shared(a,errors)
  ... check value of ...

A comment indicates a problem with real-world code:

// This test checks if dependence expressed on target and task 
// regions are honoured in the presense of nowait.
// This test is motivated by OpenMP usage in QMCPack.

[Bug libgomp/97212] [OpenMP] 'depend' clause with 'target nowait' (!) + 'task' does not work

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97212

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> ---
The testcase has:
  #pragma omp target map(tofrom: a, sum) depend(out: a) nowait
  #pragma omp task depend(in: a) shared(a)

and calls:
  __builtin_GOMP_target_ext
  __builtin_GOMP_task


The libgomp code has for "GOMP_task":

  if (!if_clause || team == NULL
      || (thr->task && thr->task->final_task)
      || team->task_count > 64 * team->nthreads)
...
      if ((flags & GOMP_TASK_FLAG_DEPEND)
          && thr->task && thr->task->depend_hash)
        gomp_task_maybe_wait_for_dependencies (depend);
...
  else
...
      if (depend_size)
        {
          gomp_task_handle_depend (task, parent, depend);
          if (task->num_dependees)
            {
              /* Tasks that depend on other tasks are not put into the
                 various waiting queues, so we are done for now.  Said
                 tasks are instead put into the queues via
                 gomp_task_run_post_handle_dependers() after their
                 dependencies have been satisfied.  After which, they
                 can be picked up by the various scheduling
                 points.  */
              gomp_mutex_unlock (&team->task_lock);
              return;
            }
        }


For the attached code, we run into the else branch, i.e. the dependency is
analyzed – a dependency is detected but then the code just returns.

There is no call to gomp_task_run_post_handle_dependers (which is only called
by  gomp_task_run_post_handle_depend which in turn is only called by
(gomp_barrier_handle_tasks and GOMP_taskwait).

The latter are "task synchronization point = A taskwait, taskgroup, or a
barrier construct." (OpenMP glossary)

Thus, the question is whether
* either the if branch should be called instead of the else branch
* or whether some task synchronization should be done after the task.

[Bug libgomp/97212] [OpenMP] 'depend' clause with 'target nowait' (!) + 'task' does not work

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97212

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I believe the testcase is invalid.
As the explicit task doesn't have if(false) clause, the task isn't undeferred,
so it is up to the implementation whether it waits for its completion or just
continues (defers it).  And there is no task synchronization point before the
if (sum != N) __builtin_abort (); statement, so if the implementation decides
to defer it, then there is no guarantee it completes before that statement.
Without the target nowait task gcc wouldn't defer it because there is just an
implicit parallel region rather than explicit (e.g. if you add parallel
num_threads(1) around most of main and change target nowait to task, you'd get
the same behavior), but in order to support async target tasks we do create
intentionally a team and thus defer the task.
So, the testcase needs something like
#pragma omp taskwait
or taskwait with depend or add if(false) to the task.

[Bug libgomp/97212] [OpenMP] 'depend' clause with 'target nowait' (!) + 'task' does not work

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97212

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Several people on omp-lang agreed on this, so closing as invalid.  I've
notified the test author.