[Openmp-commits] [PATCH] D13072: [OpenMP] Enable ThreadSanitizer to check OpenMP programs

[Jonas Hahnfeld via Openmp-commits]

Hahnfeld requested changes to this revision.
Hahnfeld added a reviewer: Hahnfeld.
Hahnfeld added a comment.
This revision now requires changes to proceed.

There are also some indention errors (especially when annotating the reductions)...



================
Comment at: runtime/src/kmp_barrier.cpp:809-815
+#ifdef DYN
+                else{
+                  for (child_tid=tid+1; child_tid<=tid+thr_bar->leaf_kids; ++child_tid) {
+                    ANNOTATE_BARRIER_AFTER(other_threads[child_tid]);
+                  }
                 }
+#endif
----------------
Is that dead code?


================
Comment at: runtime/src/kmp_tasking.c:1179-1184
 #if OMP_45_ENABLED
     // Proxy tasks are not handled by the runtime
     if ( taskdata->td_flags.proxy != TASK_PROXY )
 #endif
+    ANNOTATE_HAPPENS_AFTER(task);
     __kmp_task_start( gtid, task, current_task );
----------------
You are missing braces here, this won't work as expected!


================
Comment at: runtime/src/kmp_tasking.c:1279-1282
     if ( taskdata->td_flags.proxy != TASK_PROXY )
 #endif
+      ANNOTATE_HAPPENS_BEFORE(taskdata->td_parent);
        __kmp_task_finish( gtid, task, current_task );
----------------
You are missing braces here, this won't work as expected!


================
Comment at: runtime/src/tsan_annotations.h:86-101
+/* new higher level barrier annotations */
+#define ANNOTATE_NEW_BARRIER_BEGIN(addr) AnnotateHappensBefore(__FILE__, __LINE__, (uptr)addr)
+#define ANNOTATE_NEW_BARRIER_END(addr) AnnotateHappensAfter(__FILE__, __LINE__, (uptr)addr)
+// #define ANNOTATE_NEW_BARRIER_BEGIN(addr)
+// #define ANNOTATE_NEW_BARRIER_END(addr)
+
+/* old fine-grain barrier annotations; are replaced by higher level annotation */
----------------
What's the difference between the new and the old barrier annotations? Can the old ones be removed?


https://reviews.llvm.org/D13072