[OpenMP][Archer] Add support for taskwait depend

[protze.joachim]

Summary

At the moment Archer segfaults due to a null-pointer access, if an application uses taskwait with depend clause as used in the two new tests.
This patch cleans up the task_schedule function, moves semantic blocks into functions and replaces the if blocks by a single switch statement.
The switch statement will warn, when new enum values are added in OMPT and makes clear what code is executed for the different cases.

With free-agent tasks coming up in OpenMP 6.0, we should expect more null-pointer task_data, so additional null-pointer checks were added.
We also cannot rely on having an implicit task on the stack, so the BarrierIndex is stored during task creation.

[protze.joachim]

Applying clang-format

[Hahnfeld]

LGTM!

openmp/tools/archer/ompt-tsan.cpp

struct Taskgroup final : DataPoolEntry<Taskgroup> {
 
   static Taskgroup *New(Taskgroup *Parent) {
     return DataPoolEntry<Taskgroup>::New()->Init(Parent);
   }
 
   Taskgroup(DataPool<Taskgroup> *dp) : DataPoolEntry<Taskgroup>(dp) {}
 };
 
+enum ArcherTaskFlag { ArcherTaskFulfilled = 0x00010000 };
+
 struct TaskData;
 typedef DataPool<TaskData> TaskDataPool;
 template <> __thread TaskDataPool *TaskDataPool::ThreadDataPool = nullptr;
 
 /// Data structure to store additional information for tasks.
 struct TaskData final : DataPoolEntry<TaskData> {
   /// Its address is used for relationships of this task.
   ompt_tsan_clockid Task{0};
 
   /// Child tasks use its address to declare a relationship to a taskwait in
   /// this task.
   ompt_tsan_clockid Taskwait{0};
 
   /// Child tasks use its address to model omp_all_memory dependencies
   ompt_tsan_clockid AllMemory[2]{0};
 
+  /// Index of which barrier to use next.
+  char BarrierIndex{0};
+
   /// Whether this task is currently executing a barrier.
   bool InBarrier{false};
 
   /// Whether this task is an included task.
   int TaskType{0};
 
   /// count execution phase
   int execution{0};
 
-  /// Index of which barrier to use next.
-  char BarrierIndex{0};
-
   /// Count how often this structure has been put into child tasks + 1.
   std::atomic_int RefCount{1};
 
   /// Reference to the parent that created this task.
   TaskData *Parent{nullptr};
 
-  /// Reference to the implicit task in the stack above this task.
-  TaskData *ImplicitTask{nullptr};
-
   /// Reference to the team of this task.
   ParallelData *Team{nullptr};
 
   /// Reference to the current taskgroup that this task either belongs to or
   /// that it just created.
   Taskgroup *TaskGroup{nullptr};
 
   /// Dependency information for this task.
#endif
   bool isMergable() { return TaskType & ompt_task_mergeable; }
   bool isMerged() { return TaskType & ompt_task_merged; }
 
   bool isExplicit() { return TaskType & ompt_task_explicit; }
   bool isImplicit() { return TaskType & ompt_task_implicit; }
   bool isInitial() { return TaskType & ompt_task_initial; }
   bool isTarget() { return TaskType & ompt_task_target; }
 
+  bool isFulfilled() { return TaskType & ArcherTaskFulfilled; }
+  void setFulfilled() { TaskType |= ArcherTaskFulfilled; }
+
   void setAllMemoryDep() { AllMemory[0] = 1; }
   bool hasAllMemoryDep() { return AllMemory[0]; }
 
   void *GetTaskPtr() { return &Task; }
 
   void *GetTaskwaitPtr() { return &Taskwait; }
 
   void *GetLastAllMemoryPtr() { return AllMemory; }
   void *GetNextAllMemoryPtr() { return AllMemory + 1; }
 
   TaskData *Init(TaskData *parent, int taskType) {
     TaskType = taskType;
     Parent = parent;
     Team = Parent->Team;
+    BarrierIndex = Parent->BarrierIndex;
     if (Parent != nullptr) {
       Parent->RefCount++;
       // Copy over pointer to taskgroup. This task may set up its own stack
       // but for now belongs to its parent's taskgroup.
       TaskGroup = Parent->TaskGroup;
     }
     return this;
   }
 
   TaskData *Init(ParallelData *team, int taskType) {
     TaskType = taskType;
     execution = 1;
-    ImplicitTask = this;
     Team = team;
     return this;
   }
 
   void Reset() {
     InBarrier = false;
     TaskType = 0;
     execution = 0;
     BarrierIndex = 0;
     RefCount = 1;
     Parent = nullptr;
-    ImplicitTask = nullptr;
     Team = nullptr;
     TaskGroup = nullptr;
     if (DependencyMap) {
       for (auto i : *DependencyMap)
         i.second->Delete();
       delete DependencyMap;
     }
     DependencyMap = nullptr;
static TaskData *New(ParallelData *team, int taskType) {
     return DataPoolEntry<TaskData>::New()->Init(team, taskType);
   }
 
   TaskData(DataPool<TaskData> *dp) : DataPoolEntry<TaskData>(dp) {}
 };
 } // namespace
 
 static inline TaskData *ToTaskData(ompt_data_t *task_data) {
+  if (task_data)
     return reinterpret_cast<TaskData *>(task_data->ptr);
+  return nullptr;
 }
 
 /// Store a mutex for each wait_id to resolve race condition with callbacks.
 static std::unordered_map<ompt_wait_id_t, std::mutex> Locks;
 static std::mutex LocksMutex;
 
 static void ompt_tsan_thread_begin(ompt_thread_t thread_type,
                                    ompt_data_t *thread_data) {
if (archer_flags->all_memory) {
     else if (task->DependencyCount)
       TsanHappensAfter(task->Parent->GetLastAllMemoryPtr());
   }
   for (unsigned i = 0; i < task->DependencyCount; i++) {
     task->Dependencies[i].AnnotateBegin();
   }
 }
 
-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) {
-
-  //
-  //  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_early_fulfill)
+static void completeTask(TaskData *FromTask) {
+  if (!FromTask)
     return;
-
-  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();
-  }
-
-  // The late fulfill happens after the detached task finished execution
-  if (prior_task_status == ompt_task_late_fulfill)
+  // Task-end happens after a possible omp_fulfill_event call
+  if (FromTask->isFulfilled())
     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->isIncluded()) {
-      // Task will finish before a barrier in the surrounding parallel region
-      // ...
-      ParallelData *PData = FromTask->Team;
-      TsanHappensBefore(
-          PData->GetBarrierPtr(FromTask->ImplicitTask->BarrierIndex));
+    TsanHappensBefore(PData->GetBarrierPtr(FromTask->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
-    releaseDependencies(FromTask);
-    // free the previously running task
-    freeTask(FromTask);
-  }
 
-  // For late fulfill of detached task, there is no task to schedule to
-  if (prior_task_status == ompt_task_late_fulfill) {
+static void suspendTask(TaskData *FromTask) {
+  if (!FromTask)
     return;
-  }
+  // Task may be resumed at a later point in time.
+  TsanHappensBefore(FromTask->GetTaskPtr());
 
-  TaskData *ToTask = ToTaskData(second_task_data);
+static void switchTasks(TaskData *FromTask, TaskData *ToTask) {
   // Legacy handling for missing reduction callback
-  if (hasReductionCallback < ompt_set_always && ToTask->InBarrier) {
-    // We re-enter runtime code which currently performs a barrier.
+  if (hasReductionCallback < ompt_set_always) {
+    if (FromTask && FromTask->InBarrier) {
+      // We want to ignore writes in the runtime code during barriers,
+      // but not when executing tasks with user code!
+      TsanIgnoreWritesEnd();
+    }
+    if (ToTask && ToTask->InBarrier) {
+      // We want to ignore writes in the runtime code during barriers,
+      // but not when executing tasks with user code!
       TsanIgnoreWritesBegin();
     }
+  }
+  //// Not yet used
+  //  if (FromTask)
+  //    FromTask->deactivate();
+  //  if (ToTask)
+  //    ToTask->activate();
+}
 
-  // 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");
-  }
+static void endTask(TaskData *FromTask) {
+  if (!FromTask)
+    return;
 
+static void startTask(TaskData *ToTask) {
+  if (!ToTask)
+    return;
   // Handle dependencies on first execution of the task
   if (ToTask->execution == 0) {
     ToTask->execution++;
     acquireDependencies(ToTask);
   }
   // 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_task_schedule(ompt_data_t *first_task_data,
+                                    ompt_task_status_t prior_task_status,
+                                    ompt_data_t *second_task_data) {
+
+  //
+  //  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_taskwait_complete = 8,
+  //     -> first starts, first completes, first freed, second ignored
+  //
+  //    ompt_task_detach        = 4,
+  //    ompt_task_yield         = 2,
+  //    ompt_task_switch        = 7
+  //     -> first suspended, second starts
+  //
+
+  TaskData *FromTask = ToTaskData(first_task_data);
+  TaskData *ToTask = ToTaskData(second_task_data);
+
+  switch (prior_task_status) {
+  case ompt_task_early_fulfill:
+    TsanHappensBefore(FromTask->GetTaskPtr());
+    FromTask->setFulfilled();
+    return;
+  case ompt_task_late_fulfill:
+    TsanHappensAfter(FromTask->GetTaskPtr());
+    completeTask(FromTask);
+    freeTask(FromTask);
+    return;
+  case ompt_taskwait_complete:
+    acquireDependencies(FromTask);
+    freeTask(FromTask);
+    return;
+  case ompt_task_complete:
+    completeTask(FromTask);
+    endTask(FromTask);
+    switchTasks(FromTask, ToTask);
+    freeTask(FromTask);
+    return;
+  case ompt_task_cancel:
+    completeTask(FromTask);
+    endTask(FromTask);
+    switchTasks(FromTask, ToTask);
+    freeTask(FromTask);
+    startTask(ToTask);
+    return;
+  case ompt_task_detach:
+    endTask(FromTask);
+    suspendTask(FromTask);
+    switchTasks(FromTask, ToTask);
+    startTask(ToTask);
+    return;
+  case ompt_task_yield:
+    suspendTask(FromTask);
+    switchTasks(FromTask, ToTask);
+    startTask(ToTask);
+    return;
+  case ompt_task_switch:
+    suspendTask(FromTask);
+    switchTasks(FromTask, ToTask);
+    startTask(ToTask);
+    return;
+  }
+}
+
 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);
     if (!Data->Parent) {
       // Return since doacross dependences are not supported yet.
       return;

openmp/tools/archer/tests/races/taskwait-depend.c

+/*
+ * taskwait-depend.c -- Archer testcase
+ * derived from DRB165-taskdep4-orig-omp50-yes.c in DataRaceBench
+ */
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+//
+// See tools/archer/LICENSE.txt for details.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// RUN: %libarcher-compile-and-run-race | FileCheck %s
+// RUN: %libarcher-compile-and-run-race-noserial | FileCheck %s
+// REQUIRES: tsan
+
+#include "ompt/ompt-signal.h"
+#include <omp.h>
+#include <stdio.h>
+
+void foo() {
+
+  int x = 0, y = 2, sem = 0;
+
+#pragma omp task depend(inout : x) shared(x, sem)
+  {
+    OMPT_SIGNAL(sem);
+    x++; // 1st Child Task
+  }
+
+#pragma omp task shared(y, sem)
+  {
+    OMPT_SIGNAL(sem);
+    y--; // 2nd child task
+  }
+
+  OMPT_WAIT(sem, 2);
+#pragma omp taskwait depend(in : x) // 1st taskwait
+
+  printf("x=%d\n", x);
+  printf("y=%d\n", y);
+#pragma omp taskwait // 2nd taskwait
+}
+
+int main() {
+#pragma omp parallel num_threads(2)
+#pragma omp single
+  foo();
+
+  return 0;
+}
+
+// CHECK: WARNING: ThreadSanitizer: data race
+// CHECK-NEXT:   {{(Write|Read)}} of size 4
+// CHECK-NEXT: #0 {{.*}}taskwait-depend.c:42:20
+// CHECK:   Previous write of size 4
+// CHECK-NEXT: #0 {{.*}}taskwait-depend.c:35:6
+// CHECK: ThreadSanitizer: reported {{[0-9]+}} warnings

openmp/tools/archer/tests/task/taskwait-depend.c

+/*
+ * taskwait-depend.c -- Archer testcase
+ * derived from DRB166-taskdep4-orig-omp50-no.c in DataRaceBench
+ */
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+//
+// See tools/archer/LICENSE.txt for details.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// RUN: %libarcher-compile-and-run | FileCheck %s
+// REQUIRES: tsan
+
+#include "ompt/ompt-signal.h"
+#include <omp.h>
+#include <stdio.h>
+
+void foo() {
+
+  int x = 0, y = 2, sem = 0;
+
+#pragma omp task depend(inout : x) shared(x, sem)
+  {
+    OMPT_SIGNAL(sem);
+    x++; // 1st Child Task
+  }
+
+#pragma omp task shared(y, sem)
+  {
+    OMPT_SIGNAL(sem);
+    y--; // 2nd child task
+  }
+
+  OMPT_WAIT(sem, 2);
+#pragma omp taskwait depend(in : x) // 1st taskwait
+
+  printf("x=%d\n", x);
+
+#pragma omp taskwait // 2nd taskwait
+
+  printf("y=%d\n", y);
+}
+
+int main() {
+#pragma omp parallel num_threads(2)
+#pragma omp single
+  foo();
+
+  return 0;
+}
+
+// CHECK-NOT: ThreadSanitizer: data race
+// CHECK-NOT: ThreadSanitizer: reported
+// CHECK: y=1