[Openmp-commits] [PATCH] D55577: [OMPT] First chunk of final OMPT 5.0 interface updates

[Hansang Bae via Phabricator via Openmp-commits]

hbae added inline comments.


================
Comment at: runtime/src/include/50/ompt.h.var:56
 
 #define FOREACH_OMP_STATE(macro)                                                                \
                                                                                                 \
----------------
I think it's better to change this macro name (to `FOREACH_OMPT_STATE`) as well.


================
Comment at: runtime/src/include/50/ompt.h.var:182
+typedef uint64_t ompt_wait_id_t;
+static const ompt_wait_id_t omp_wait_id_none = 0;
 
----------------
Can we `#define` this variable?


================
Comment at: runtime/src/kmp_csupport.cpp:510
+      this_thr->th.ompt_thread_info.state != ompt_state_overhead) {
+    OMPT_CUR_TASK_INFO(this_thr)->frame.exit_frame.ptr = NULL;
     if (ompt_enabled.ompt_callback_implicit_task) {
----------------
I think initializing `exit_frame` with `ompt_data_none` is safer.
There are several places in the patch that do the similar assignment with NULL or 0.


================
Comment at: runtime/src/kmp_gsupport.cpp:1087
   if (ompt_enabled.enabled) {                                                  \
-    parent_frame->enter_frame = NULL;                                          \
+    parent_frame->enter_frame.ptr = NULL;                                          \
   }
----------------
Can you align the backslashes in the patch?


Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D55577