[Openmp-commits] [PATCH] D38185: Implementation of OMPT as specified in OpenMP 5.0 Preview 1

[Olga Malysheva via Phabricator via Openmp-commits]

omalyshe added inline comments.


================
Comment at: runtime/src/include/50/omp_lib.f.var:619
 
+!dec$ attributes alias:'OMP_CONTROL_TOOL' :: omp_control_tool
+
----------------
Move up where all OMP_* are located


================
Comment at: runtime/src/include/50/omp_lib.f.var:701
 
+!dec$ attributes alias:'_OMP_CONTROL_TOOL' :: omp_control_tool
+
----------------
Move up where all _OMP_* are located


================
Comment at: runtime/src/include/50/omp_lib.f.var:785
 
+!dec$ attributes alias:'omp_control_tool_'::omp_control_tool
+
----------------
Move up as well


================
Comment at: runtime/src/include/50/omp_lib.f.var:867
 
+!dec$ attributes alias:'_omp_control_tool_'::omp_control_tool
+
----------------
Move up


================
Comment at: runtime/src/include/50/ompt.h.var:82
+    /* target wait states (128..255) */                                                         \
+    macro (omp_state_wait_target, 0x080)        /* waiting for critical */                      \
+    macro (omp_state_wait_target_map, 0x081)    /* waiting for atomic */                        \
----------------
Three comments copy-pasted?


================
Comment at: runtime/src/include/50/ompt.h.var:311
+    ompt_invoker_t invoker,           /* who invokes master task?     */
+    const void *codeptr_ra            
 );
----------------
Need a comment at least once?


================
Comment at: runtime/src/include/50/ompt.h.var:341
+
+typedef void (*ompt_callback_task_schedule_t) (
+    ompt_data_t *first_task_data,
----------------
Need description of fields?


================
Comment at: runtime/src/include/50/ompt.h.var:367
 );
 
+typedef enum ompt_target_type_e {
----------------
/* target and device */


================
Comment at: runtime/src/include/50/ompt.h.var:386
+    ompt_target_data_alloc = 1,
+    ompt_target_data_transfer_to_dev = 2,
+    ompt_target_data_transfer_from_dev = 3,
----------------
I suggest using *_device instead of *_dev


================
Comment at: runtime/src/include/50/ompt.h.var:415
+/* control_tool */
+typedef int (*ompt_callback_control_tool_t) (
     uint64_t command,                 /* command of control call      */
----------------
Move after device_finalize


================
Comment at: runtime/src/include/50/ompt.h.var:512
+    ompt_cancel_sections       = 0x2,
+    ompt_cancel_do             = 0x4,
+    ompt_cancel_taskgroup      = 0x8,
----------------
Probably, ompt_cancel_loop is better


================
Comment at: runtime/src/include/50/ompt.h.var:654
 
 typedef enum opt_init_mode_e {
     ompt_init_mode_never  = 0,
----------------
typo: should be "ompt_init_mode..."


https://reviews.llvm.org/D38185