[OpenMP][Tool] Handle detached tasks in Archer

[protze.joachim]

Summary

Since detached tasks are supported by clang and the OpenMP runtime, Archer must expect to receive the corresponding callbacks.

This patch adds support to interpret the synchronization semantics of omp_fulfill_event and cleans up the handling of task switches.
The task_late_fulfill.c currently cannot use if(0) for the first task to avoid the race on f_event.

[protze.joachim]

The tests of cause require tsan.
Also added build dependence to tsan, if tsan is built in the same build.

[protze.joachim]

ping

[protze.joachim]

Fixed handling of ompt_task_cancel.

[jdoerfert]

This looks reasonable, not that I have much experience with ARCHER.

openmp/tools/archer/tests/task/task_late_fulfill.c

39

Leftover?

[protze.joachim]

Pushed as e99207feb4b901e8f7bb6d3e70388d31fafc4330, missed to include the Revision line.

[ye-luo]

openmp/tools/archer/tests/task/task_late_fulfill.c

37

de-referencing uninitialized pointer f_event? I expect no task has been run at this point.

[ye-luo]

openmp/tools/archer/tests/task/task_late_fulfill.c

37

de-referencing uninitialized pointer f_event? I expect no task has been run at this point.

This test is probably good.
When OMP_NUM_THREADS>1, there are additional threads available for picking up task 1 and task must be completed within 20000 usec. Not robust but probably OK.
When OMP_NUM_THREADS=1, the parallel is inactive and task 1 runs immediately, task 2 is deferred due to dependency, So this test also works fine.

[protze.joachim]

Original intention was to have if(0) on the first task. Unfortunately, clang drops the out dependency for detached tasks in that case: https://bugs.llvm.org/show_bug.cgi?id=46185
Once the clang bug is fixed, we can add the if(0) and make the test more robust.

[protze.joachim]

openmp/tools/archer/tests/task/task_late_fulfill.c

37

During discussion for the OpenMP Examples document, I realized that taking the address is not necessary. I pushed a fix as 3fa2e193384048daafe1fa74bf88253b4cf5ce6e.

openmp/tools/archer/ompt-tsan.cpp

if (type & ompt_task_initial) {
     // Use the newly created address. We cannot use a single address from the
     // parent because that would declare wrong relationships with other
     // sibling tasks that may be created before this task is started!
     TsanHappensBefore(Data->GetTaskPtr());
     ToTaskData(parent_task_data)->execution++;
   }
 }
 
-static void ompt_tsan_task_schedule(ompt_data_t *first_task_data,
-                                    ompt_task_status_t prior_task_status,
-                                    ompt_data_t *second_task_data) {
-  TaskData *FromTask = ToTaskData(first_task_data);
-  TaskData *ToTask = ToTaskData(second_task_data);
-
-  if (ToTask->Included && prior_task_status != ompt_task_complete)
-    return; // No further synchronization for begin included tasks
-  if (FromTask->Included && prior_task_status == ompt_task_complete) {
-    // Just delete the task:
-    while (FromTask != nullptr && --FromTask->RefCount == 0) {
-      TaskData *Parent = FromTask->Parent;
-      if (FromTask->DependencyCount > 0) {
-        delete[] FromTask->Dependencies;
-      }
-      delete FromTask;
-      FromTask = Parent;
-    }
-    return;
-  }
+static void __ompt_tsan_release_task(TaskData *task) {
+  while (task != nullptr && --task->RefCount == 0) {
+    TaskData *Parent = task->Parent;
+    if (task->DependencyCount > 0) {
+      delete[] task->Dependencies;
+    delete task;
+    task = Parent;
 
-  if (ToTask->execution == 0) {
-    ToTask->execution++;
-    // 1. Task will begin execution after it has been created.
-    TsanHappensAfter(ToTask->GetTaskPtr());
-    for (unsigned i = 0; i < ToTask->DependencyCount; i++) {
-      ompt_dependence_t *Dependency = &ToTask->Dependencies[i];
+static void ompt_tsan_task_schedule(ompt_data_t *first_task_data,
+                                    ompt_task_status_t prior_task_status,
+                                    ompt_data_t *second_task_data) {
 
-      TsanHappensAfter(Dependency->variable.ptr);
-      // in and inout dependencies are also blocked by prior in dependencies!
-      if (Dependency->dependence_type == ompt_dependence_type_out || Dependency->dependence_type == ompt_dependence_type_inout) {
-        TsanHappensAfter(ToInAddr(Dependency->variable.ptr));
-      }
-    }
-  } else {
-    // 2. Task will resume after it has been switched away.
-    TsanHappensAfter(ToTask->GetTaskPtr());
-  }
+  //
+  //  The necessary action depends on prior_task_status:
+  //
+  //    ompt_task_early_fulfill = 5,
+  //     -> ignored
+  //
+  //    ompt_task_late_fulfill  = 6,
+  //     -> first completed, first freed, second ignored
+  //
+  //    ompt_task_complete      = 1,
+  //    ompt_task_cancel        = 3,
+  //     -> first completed, first freed, second starts
+  //
+  //    ompt_task_detach        = 4,
+  //    ompt_task_yield         = 2,
+  //    ompt_task_switch        = 7
+  //     -> first suspended, second starts
+  //
 
-  if (prior_task_status != ompt_task_complete) {
-    ToTask->ImplicitTask = FromTask->ImplicitTask;
-    assert(ToTask->ImplicitTask != NULL &&
-           "A task belongs to a team and has an implicit task on the stack");
-  }
+  if (prior_task_status == ompt_task_early_fulfill)
+    return;
 
-  // Task may be resumed at a later point in time.
-  TsanHappensBefore(FromTask->GetTaskPtr());
+  TaskData *FromTask = ToTaskData(first_task_data);
 
+  // Legacy handling for missing reduction callback
   if (hasReductionCallback < ompt_set_always && FromTask->InBarrier) {
     // We want to ignore writes in the runtime code during barriers,
     // but not when executing tasks with user code!
     TsanIgnoreWritesEnd();
   }
 
-  if (prior_task_status == ompt_task_complete) { // task finished
-
-    // Task will finish before a barrier in the surrounding parallel region ...
+  // The late fulfill happens after the detached task finished execution
+  if (prior_task_status == ompt_task_late_fulfill)
+    TsanHappensAfter(FromTask->GetTaskPtr());
+
+  // task completed execution
+  if (prior_task_status == ompt_task_complete ||
+      prior_task_status == ompt_task_cancel ||
+      prior_task_status == ompt_task_late_fulfill) {
+    // Included tasks are executed sequentially, no need to track
+    // synchronization
+    if (!FromTask->Included) {
+      // Task will finish before a barrier in the surrounding parallel region
+      // ...
       ParallelData *PData = FromTask->Team;
       TsanHappensBefore(
           PData->GetBarrierPtr(FromTask->ImplicitTask->BarrierIndex));
 
       // ... and before an eventual taskwait by the parent thread.
       TsanHappensBefore(FromTask->Parent->GetTaskwaitPtr());
 
       if (FromTask->TaskGroup != nullptr) {
         // This task is part of a taskgroup, so it will finish before the
         // corresponding taskgroup_end.
         TsanHappensBefore(FromTask->TaskGroup->GetPtr());
       }
+    }
+
+    // release dependencies
     for (unsigned i = 0; i < FromTask->DependencyCount; i++) {
       ompt_dependence_t *Dependency = &FromTask->Dependencies[i];
 
       // in dependencies block following inout and out dependencies!
       TsanHappensBefore(ToInAddr(Dependency->variable.ptr));
       if (Dependency->dependence_type == ompt_dependence_type_out || Dependency->dependence_type == ompt_dependence_type_inout) {
         TsanHappensBefore(Dependency->variable.ptr);
       }
     }
-    while (FromTask != nullptr && --FromTask->RefCount == 0) {
-      TaskData *Parent = FromTask->Parent;
-      if (FromTask->DependencyCount > 0) {
-        delete[] FromTask->Dependencies;
-      }
-      delete FromTask;
-      FromTask = Parent;
-    }
+    // free the previously running task
+    __ompt_tsan_release_task(FromTask);
+
+  // For late fulfill of detached task, there is no task to schedule to
+  if (prior_task_status == ompt_task_late_fulfill) {
+    return;
   }
+
+  TaskData *ToTask = ToTaskData(second_task_data);
+  // Legacy handling for missing reduction callback
   if (hasReductionCallback < ompt_set_always && ToTask->InBarrier) {
     // We re-enter runtime code which currently performs a barrier.
     TsanIgnoreWritesBegin();
   }
+
+  // task suspended
+  if (prior_task_status == ompt_task_switch ||
+      prior_task_status == ompt_task_yield ||
+      prior_task_status == ompt_task_detach) {
+    // Task may be resumed at a later point in time.
+    TsanHappensBefore(FromTask->GetTaskPtr());
+    ToTask->ImplicitTask = FromTask->ImplicitTask;
+    assert(ToTask->ImplicitTask != NULL &&
+           "A task belongs to a team and has an implicit task on the stack");
+  }
+
+  // Handle dependencies on first execution of the task
+  if (ToTask->execution == 0) {
+    ToTask->execution++;
+    for (unsigned i = 0; i < ToTask->DependencyCount; i++) {
+      ompt_dependence_t *Dependency = &ToTask->Dependencies[i];
+
+      TsanHappensAfter(Dependency->variable.ptr);
+      // in and inout dependencies are also blocked by prior in dependencies!
+      if (Dependency->dependence_type == ompt_dependence_type_out ||
+          Dependency->dependence_type == ompt_dependence_type_inout) {
+        TsanHappensAfter(ToInAddr(Dependency->variable.ptr));
+      }
+    }
+  }
+  // 1. Task will begin execution after it has been created.
+  // 2. Task will resume after it has been switched away.
+  TsanHappensAfter(ToTask->GetTaskPtr());
 }
 
 static void ompt_tsan_dependences(ompt_data_t *task_data,
                                   const ompt_dependence_t *deps,
                                   int ndeps) {
   if (ndeps > 0) {
     // Copy the data to use it in task_switch and task_end.
     TaskData *Data = ToTaskData(task_data);

openmp/tools/archer/tests/CMakeLists.txt

macro(pythonize_bool var)
   else()
     set(${var} False)
   endif()
 endmacro()
 
 pythonize_bool(LIBARCHER_HAVE_LIBATOMIC)
 pythonize_bool(OPENMP_TEST_COMPILER_HAS_TSAN_FLAGS)
 
-add_openmp_testsuite(check-libarcher "Running libarcher tests" ${CMAKE_CURRENT_BINARY_DIR} DEPENDS archer omp)
+set(ARCHER_TSAN_TEST_DEPENDENCE "")
+if(TARGET tsan)
+  set(ARCHER_TSAN_TEST_DEPENDENCE tsan)
+endif()
+
+add_openmp_testsuite(check-libarcher "Running libarcher tests" ${CMAKE_CURRENT_BINARY_DIR} 
+                     DEPENDS archer omp ${ARCHER_TSAN_TEST_DEPENDENCE})
 
 # Configure the lit.site.cfg.in file
 set(AUTO_GEN_COMMENT "## Autogenerated by libarcher configuration.\n# Do not edit!")
 configure_file(lit.site.cfg.in lit.site.cfg @ONLY)

openmp/tools/archer/tests/task/task_early_fulfill.c

+// RUN: %libarcher-compile -fopenmp-version=50 && env OMP_NUM_THREADS='3' \
+// RUN:    %libarcher-run
+//| FileCheck %s
+
+// Checked gcc 9.2 still does not support detach clause on task construct.
+// UNSUPPORTED: gcc-4, gcc-5, gcc-6, gcc-7, gcc-8, gcc-9
+// clang supports detach clause since version 11.
+// UNSUPPORTED: clang-10, clang-9, clang-8, clang-7
+// icc compiler does not support detach clause.
+// UNSUPPORTED: icc
+// REQUIRES: tsan
+
+#include <omp.h>
+#include <stdio.h>
+
+int main() {
+#pragma omp parallel
+#pragma omp master
+  {
+    omp_event_handle_t event;
+#pragma omp task detach(event) if (0)
+    { omp_fulfill_event(event); }
+#pragma omp taskwait
+  }
+  return 0;
+}

openmp/tools/archer/tests/task/task_late_fulfill.c

+// RUN: %libarcher-compile -fopenmp-version=50 && env OMP_NUM_THREADS='3' \
+// RUN:   %libarcher-run-race | FileCheck %s
+
+// Checked gcc 9.2 still does not support detach clause on task construct.
+// UNSUPPORTED: gcc-4, gcc-5, gcc-6, gcc-7, gcc-8, gcc-9
+// clang supports detach clause since version 11.
+// UNSUPPORTED: clang-10, clang-9, clang-8, clang-7
+// icc compiler does not support detach clause.
+// UNSUPPORTED: icc
+// REQUIRES: tsan
+
+#include <omp.h>
+#include <stdio.h>
+#include <unistd.h>
+
+int main() {
+#pragma omp parallel
+#pragma omp master
+  {
+    omp_event_handle_t event;
+    int a = 0, b = 0;
+    omp_event_handle_t *f_event;
+#pragma omp task detach(event) depend(out : f_event) shared(f_event)
+    {
+      printf("%i: task 1\n", omp_get_thread_num());
+      f_event = &event;
+    }
+    usleep(10000);
+#pragma omp task depend(in : f_event) shared(f_event, a, b)
+    {
+      printf("%i: task 2, %p, %i, %i\n", omp_get_thread_num(), f_event, a, b);
+      f_event = &event;
+    }
+    usleep(10000);
+    a++;
+    printf("%i: calling omp_fulfill_event\n", omp_get_thread_num());
+    omp_fulfill_event(*f_event);
+//#pragma omp task if (0) depend(in : f_event)
+//    {}
+    b++;
+    usleep(10000);
+#pragma omp taskwait
+  }
+  return 0;
+}
+
+// no race for a++ in line 32:
+// CHECK-NOT: #0 {{.*}}task_late_fulfill.c:35
+
+// we expect a race on f_event:
+
+// CHECK: WARNING: ThreadSanitizer: data race
+// CHECK-NEXT:   {{(Write|Read)}} of size 8
+// CHECK-NEXT: #0 {{.*}}task_late_fulfill.c:37
+// CHECK:   Previous write of size 8
+// CHECK-NEXT: #0 {{.*}}task_late_fulfill.c:26
+
+// CHECK: WARNING: ThreadSanitizer: data race
+// CHECK-NEXT:   {{(Write|Read)}} of size 4
+// CHECK-NEXT: #0 {{.*}}task_late_fulfill.c:31
+// CHECK:   Previous write of size 4
+// CHECK-NEXT: #0 {{.*}}task_late_fulfill.c:40