[Openmp-commits] [PATCH] D100181: [OpenMP] [OMPD] [1/6] Implementation of OMPD debugging library - libompd. Code changes in openmp/runtime to support libompd.

[Jini Susan George via Phabricator via Openmp-commits]

jini.susan added inline comments.


================
Comment at: openmp/runtime/src/include/omp-tools.h.var:1307
+ompd_rc_t ompd_get_state(ompd_thread_handle_t *thread_handle,
+                         ompd_word_t *state, ompt_wait_id_t *wait_id);
+
----------------
hbae wrote:
> We need to use `ompd_wait_id_t`.
Thanks for this good catch! Will work on modifying this.


================
Comment at: openmp/runtime/src/kmp_runtime.cpp:2050
     propagateFPControl(team);
+#if OMPD_SUPPORT
+    if (ompd_state & OMPD_ENABLE_BP)
----------------
hbae wrote:
> We already have program points defined for OMPT callbacks.
> Isn't it better to place parallel begin/end BP at the places near OMPT parallel begin/end?
> Are there any missing parallel begin/end not covered by the current OMPT parallel begin/end?
According to the spec, there is a difference in the binding associated with the current parallel handle with the OMPT parallel-begin event and the OMPD parallel-begin event. 

For OMPD, (Section 5.6.1.)

The OpenMP implementation must execute ompd_bp_parallel_begin at every
parallel-begin event. At the point that the implementation reaches
ompd_bp_parallel_begin, the binding for ompd_get_curr_parallel_handle is the
parallel region that is beginning and the binding for ompd_get_curr_task_handle is the
task that encountered the parallel construct.

For OMPT, 

Between a parallel-begin event and an implicit-task-begin event, a call to
ompt_get_parallel_info(0,...) may return information about the outer parallel team,
the new parallel team or an inconsistent state.

The OMPD runtime entry points in this implementation are invoked at points to ensure that the right binding holds as per the spec.



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

https://reviews.llvm.org/D100181