[Openmp-commits] [PATCH] D103608: [OpenMP][Tools] Fix Archer handling of task dependencies

[Joachim Protze via Phabricator via Openmp-commits]

protze.joachim added inline comments.


================
Comment at: openmp/tools/archer/ompt-tsan.cpp:887-897
+static void __ompt_tsan_release_dependencies(TaskData *task) {
+  for (unsigned i = 0; i < task->DependencyCount; i++) {
+    task->Dependencies[i].AnnotateEnd();
+  }
+}
+
+static void __ompt_tsan_acquire_dependencies(TaskData *task) {
----------------
Hahnfeld wrote:
> Can we name internal functions without double underscores, which are actually not allowed by the standard? (`__ompt_tsan_release_task` should be renamed eventually) Also I understand release + acquire semantics, but "release" is actually a bit confusing because it's not about actually releasing the memory AFAICT. I understand it already was confusing before, but if somebody has a better proposal for naming (I don't right now) that would be great.
Since we a mid-term goal should be to refactor towards LLVM coding style, I changed the function names to use camelCase as required for function names.

I could live with renaming the task memory release function (`__ompt_tsan_release_task`) to `freeTask`.
The acquire/release refers to the similar semantics of locks.


================
Comment at: openmp/tools/archer/ompt-tsan.cpp:920
   if (ToTask->execution == 0) {
-    ToTask->execution++;
-    for (unsigned i = 0; i < ToTask->DependencyCount; i++) {
----------------
Hahnfeld wrote:
> Is this removed on purpose?
Good catch! I missed this intermediate change in separating the changes into multiple patches.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103608/new/

https://reviews.llvm.org/D103608