Do not block on explicit task depending on proxy task

[Hahnfeld]

Summary

Consider the following code:

int dep;
#pragma omp target nowait depend(out: dep)
{
    sleep(1);
}
#pragma omp task depend(in: dep)
{
    printf("Task with dependency\n");
}
printf("Doing some work...\n");

In its current state the runtime will block on the second task and not
continue execution.

[hfinkel]

Please add your example as a regression test case.

[Hahnfeld]

Add test emulating what I think the Intel Compiler generates and which shows the issue

[AndreyChurbanov]

Jonas,

What if you add one more dependent task (not on the proxy task)? E.g.

int dep;
#pragma omp target nowait depend(out: dep)
{
    sleep(1);
}
#pragma omp task depend(in: dep)
{
    printf("Task with dependency\n");
}
#pragma omp task depend(out: dep)
{
    printf("Another task with dependency\n");
}
printf("Doing some work...\n");

The second dependent task will be lost (unless it also has "in" dependency and thus only depends on the proxy task). It should not be hard to allow all serial dependent tasks be deferred in the presence of proxy task, just couple more changes in the

__kmp_task_finish()

routine can solve this I think. But is it what you want? If you want to only allow tasks those depend on the proxy task be deferred letting other serial dependent tasks be undeferred, then more changes may be needed. Library will need to distinguish tasks dependent on proxy task (among possible multiple dependencies) from tasks dependent on only regular tasks.

[Hahnfeld]

Fix for dependency chain with proxy task as its origin

[Hahnfeld]

In D23116#505993, @AndreyChurbanov wrote:

Jonas,

What if you add one more dependent task (not on the proxy task)? E.g.

int dep;
#pragma omp target nowait depend(out: dep)
{
    sleep(1);
}
#pragma omp task depend(in: dep)
{
    printf("Task with dependency\n");
}
#pragma omp task depend(out: dep)
{
    printf("Another task with dependency\n");
}
printf("Doing some work...\n");

The second dependent task will be lost (unless it also has "in" dependency and thus only depends on the proxy task). It should not be hard to allow all serial dependent tasks be deferred in the presence of proxy task, just couple more changes in the

__kmp_task_finish()

routine can solve this I think. But is it what you want? If you want to only allow tasks those depend on the proxy task be deferred letting other serial dependent tasks be undeferred, then more changes may be needed. Library will need to distinguish tasks dependent on proxy task (among possible multiple dependencies) from tasks dependent on only regular tasks.

Good catch, thanks! I think I've modified __kmp_task_finish() as you said to release eventual deps and added another dependent task to the test.

I think this will only allow dependency chains with a proxy task as its origin. Otherwise the previous tasks in the serial team will already have finished.
This probably means some no-op checking for serial tasks with dependencies after a proxy task has occured but I think that this is better than blocking on a dependent task.

Thanks,
Jonas

[AndreyChurbanov]

LGTM

runtime/src/kmp_taskdeps.cpp

if (ompt_enabled && ndeps+ndeps_noalias > 0 &&
                 new_taskdata->ompt_task_info.deps[ndeps+i].dependence_flags =
                   ompt_task_dependence_type_in;
         }
     }
 #endif /* OMPT_SUPPORT && OMPT_TRACE */
 
     bool serial = current_task->td_flags.team_serial || current_task->td_flags.tasking_ser || current_task->td_flags.final;
 #if OMP_45_ENABLED
-    serial = serial && !(new_taskdata->td_flags.proxy == TASK_PROXY);
+    kmp_task_team_t * task_team = thread->th.th_task_team;
+    serial = serial && !(task_team && task_team->tt.tt_found_proxy_tasks);
 #endif
 
     if ( !serial && ( ndeps > 0 || ndeps_noalias > 0 )) {
         /* if no dependencies have been tracked yet, create the dependence hash */
         if ( current_task->td_dephash == NULL )
             current_task->td_dephash = __kmp_dephash_create(thread, current_task);
 
 #if USE_FAST_MEMORY
         kmp_depnode_t *node = (kmp_depnode_t *) __kmp_fast_allocate(thread,sizeof(kmp_depnode_t));
 #else
         kmp_depnode_t *node = (kmp_depnode_t *) __kmp_thread_malloc(thread,sizeof(kmp_depnode_t));
 #endif
 
         __kmp_init_node(node);
         new_taskdata->td_depnode = node;
 
         if ( __kmp_check_deps( gtid, node, new_task, current_task->td_dephash, NO_DEP_BARRIER,
                                ndeps, dep_list, ndeps_noalias,noalias_dep_list ) ) {
             KA_TRACE(10, ("__kmpc_omp_task_with_deps(exit): T#%d task had blocking dependencies: "
                   "loc=%p task=%p, return: TASK_CURRENT_NOT_QUEUED\n", gtid, loc_ref,
                   new_taskdata ) );
             return TASK_CURRENT_NOT_QUEUED;
         }
     } else {
-#if OMP_45_ENABLED
-        kmp_task_team_t * task_team = thread->th.th_task_team;
-        if ( task_team && task_team->tt.tt_found_proxy_tasks )
-           __kmpc_omp_wait_deps ( loc_ref, gtid, ndeps, dep_list, ndeps_noalias, noalias_dep_list );
-        else
-#endif
-           KA_TRACE(10, ("__kmpc_omp_task_with_deps(exit): T#%d ignored dependencies for task (serialized)"
-                           "loc=%p task=%p\n", gtid, loc_ref, new_taskdata ) );
     }
 
     KA_TRACE(10, ("__kmpc_omp_task_with_deps(exit): T#%d task had no blocking dependencies : "
                   "loc=%p task=%p, transferring to __kmpc_omp_task\n", gtid, loc_ref,
                   new_taskdata ) );
 
     return __kmpc_omp_task(loc_ref,gtid,new_task);
 }

runtime/src/kmp_tasking.c

 // task: task to be finished
 // resumed_task: task to be resumed.  (may be NULL if task is serialized)
 
 static void
 __kmp_task_finish( kmp_int32 gtid, kmp_task_t *task, kmp_taskdata_t *resumed_task )
 {
     kmp_taskdata_t * taskdata = KMP_TASK_TO_TASKDATA(task);
     kmp_info_t * thread = __kmp_threads[ gtid ];
+    kmp_task_team_t * task_team = thread->th.th_task_team; // might be NULL for serial teams...
     kmp_int32 children = 0;
 
 #if OMPT_SUPPORT
     if (ompt_enabled &&
         ompt_callbacks.ompt_callback(ompt_event_task_end)) {
         kmp_taskdata_t *parent = taskdata->td_parent;
         ompt_callbacks.ompt_callback(ompt_event_task_end)(
             taskdata->ompt_task_info.task_id);
#endif /* BUILD_TIED_TASK_STACK */
     // Only need to keep track of count if team parallel and tasking not serialized
     if ( !( taskdata -> td_flags.team_serial || taskdata -> td_flags.tasking_ser ) ) {
         // Predecrement simulated by "- 1" calculation
         children = KMP_TEST_THEN_DEC32( (kmp_int32 *)(& taskdata -> td_parent -> td_incomplete_child_tasks) ) - 1;
         KMP_DEBUG_ASSERT( children >= 0 );
 #if OMP_40_ENABLED
         if ( taskdata->td_taskgroup )
             KMP_TEST_THEN_DEC32( (kmp_int32 *)(& taskdata->td_taskgroup->count) );
+#if OMP_45_ENABLED
+    }
+    // if we found proxy tasks there could exist a dependency chain
+    // with the proxy task as origin
+    if ( !( taskdata -> td_flags.team_serial || taskdata -> td_flags.tasking_ser ) || (task_team && task_team->tt.tt_found_proxy_tasks) ) {
+#endif
         __kmp_release_deps(gtid,taskdata);
 #endif
     }
 
     // td_flags.executing  must be marked as 0 after __kmp_release_deps has been called
     // Othertwise, if a task is executed immediately from the release_deps code
     // the flag will be reset to 1 again by this same function
     KMP_DEBUG_ASSERT( taskdata -> td_flags.executing == 1 );
#endif // OMP_40_ENABLED
     // GEH - note tasking_ser => task_serial
     KMP_DEBUG_ASSERT( (taskdata->td_flags.tasking_ser || taskdata->td_flags.task_serial) ==
                        taskdata->td_flags.task_serial);
     if ( taskdata->td_flags.task_serial )
     {
         if (resumed_task == NULL) {
             resumed_task = taskdata->td_parent;  // In a serialized task, the resumed task is the parent
         }
-        else {
+        else
+#if OMP_45_ENABLED
+             if ( !(task_team && task_team->tt.tt_found_proxy_tasks) )
+#endif
+        {
             // verify resumed task passed in points to parent
             KMP_DEBUG_ASSERT( resumed_task == taskdata->td_parent );
         }
     }
     else {
         KMP_DEBUG_ASSERT( resumed_task != NULL );        // verify that resumed task is passed as arguemnt
     }
 

runtime/test/tasking/bug_proxy_task_dep_waiting.c

+// RUN: %libomp-compile -lpthread && %libomp-run
+#include <stdio.h>
+#include <omp.h>
+#include <pthread.h>
+#include "omp_my_sleep.h"
+
+/*
+ An explicit task can have a dependency on a target task. If it is not
+ directly satisfied, the runtime should not wait but resume execution.
+*/
+
+// Compiler-generated code (emulation)
+typedef long kmp_intptr_t;
+typedef int kmp_int32;
+
+typedef char bool;
+
+typedef struct ident {
+    kmp_int32 reserved_1;   /**<  might be used in Fortran; see above  */
+    kmp_int32 flags;        /**<  also f.flags; KMP_IDENT_xxx flags; KMP_IDENT_KMPC identifies this union member  */
+    kmp_int32 reserved_2;   /**<  not really used in Fortran any more; see above */
+#if USE_ITT_BUILD
+                            /*  but currently used for storing region-specific ITT */
+                            /*  contextual information. */
+#endif /* USE_ITT_BUILD */
+    kmp_int32 reserved_3;   /**< source[4] in Fortran, do not use for C++  */
+    char const *psource;    /**< String describing the source location.
+                            The string is composed of semi-colon separated fields which describe the source file,
+                            the function and a pair of line numbers that delimit the construct.
+                             */
+} ident_t;
+
+typedef struct kmp_depend_info {
+     kmp_intptr_t               base_addr;
+     size_t                     len;
+     struct {
+         bool                   in:1;
+         bool                   out:1;
+     } flags;
+} kmp_depend_info_t;
+
+struct kmp_task;
+typedef kmp_int32 (* kmp_routine_entry_t)( kmp_int32, struct kmp_task * );
+
+typedef struct kmp_task {                   /* GEH: Shouldn't this be aligned somehow? */
+    void *              shareds;            /**< pointer to block of pointers to shared vars   */
+    kmp_routine_entry_t routine;            /**< pointer to routine to call for executing task */
+    kmp_int32           part_id;            /**< part id for the task                          */
+} kmp_task_t;
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+kmp_int32  __kmpc_global_thread_num  ( ident_t * );
+kmp_task_t*
+__kmpc_omp_task_alloc( ident_t *loc_ref, kmp_int32 gtid, kmp_int32 flags,
+                       size_t sizeof_kmp_task_t, size_t sizeof_shareds,
+                       kmp_routine_entry_t task_entry );
+void __kmpc_proxy_task_completed_ooo ( kmp_task_t *ptask );
+kmp_int32 __kmpc_omp_task_with_deps ( ident_t *loc_ref, kmp_int32 gtid, kmp_task_t * new_task,
+                                      kmp_int32 ndeps, kmp_depend_info_t *dep_list,
+                                      kmp_int32 ndeps_noalias, kmp_depend_info_t *noalias_dep_list );
+kmp_int32
+__kmpc_omp_task( ident_t *loc_ref, kmp_int32 gtid, kmp_task_t * new_task );
+#ifdef __cplusplus
+}
+#endif
+
+void *target(void *task)
+{
+    my_sleep( 0.1 );
+    __kmpc_proxy_task_completed_ooo((kmp_task_t*) task);
+    return NULL;
+}
+
+pthread_t target_thread;
+
+// User's code
+int task_entry(kmp_int32 gtid, kmp_task_t *task)
+{
+    pthread_create(&target_thread, NULL, &target, task);
+    return 0;
+}
+
+int main()
+{
+    int dep;
+
+/*
+ *  Corresponds to:
+    #pragma omp target nowait depend(out: dep)
+    {
+        my_sleep( 0.1 );
+    }
+*/
+    kmp_depend_info_t dep_info;
+    dep_info.base_addr = (long) &dep;
+    dep_info.len = sizeof(int);
+    // out = inout per spec and runtime expects this
+    dep_info.flags.in = 1;
+    dep_info.flags.out = 1;
+
+    kmp_int32 gtid = __kmpc_global_thread_num(NULL);
+    kmp_task_t *proxy_task = __kmpc_omp_task_alloc(NULL,gtid,17,sizeof(kmp_task_t),0,&task_entry);
+    __kmpc_omp_task_with_deps(NULL,gtid,proxy_task,1,&dep_info,0,NULL);
+
+    int first_task_finished = 0;
+    #pragma omp task shared(first_task_finished) depend(inout: dep)
+    {
+        first_task_finished = 1;
+    }
+
+    int second_task_finished = 0;
+    #pragma omp task shared(second_task_finished) depend(in: dep)
+    {
+        second_task_finished = 1;
+    }
+
+    // check that execution has been resumed and the runtime has not waited
+    // for the dependencies to be satisfied.
+    int error = (first_task_finished == 1);
+    error += (second_task_finished == 1);
+
+    #pragma omp taskwait
+
+    // by now all tasks should have finished
+    error += (first_task_finished != 1);
+    error += (second_task_finished != 1);
+
+    return error;
+}