[OMPT] Rename ompt_mutex_impl_t to kmp_mutex_impl

[sconvent]

Summary

The defintion is not part of the spec and thus should not have the prefix "ompt_" but rather a prefix that indicates that this is implementation specific.

[Hahnfeld]

Makes sense in some way. However if I read the spec correctly the tool gets an unsigned int. So maybe we should keep the enum in the header but prefix it with kmp instead of ompt to make clear that this is not portable? This would allow tools to conditionally compare the passed value

[sconvent]

Implemented suggestion

[Hahnfeld]

runtime/src/include/50/ompt.h.var

87 ↗

(On Diff #127842)

This is actually defined in the spec, so we need to keep it prefixed with ompt

[sconvent]

Implemented suggestion

[sconvent]

runtime/src/include/50/ompt.h.var

87 ↗

(On Diff #127842)

You're right. The spec specifies this value as a macro however (not an enum). I think we should wait for feedback on Github Issue 112

[protze.joachim]

We discussed to relax the wording in the OpenMP spec, so that any implementation that provides ompt_mutex_impl_unknown with value 0 is complying. This will include #define, enum or "static const" values.

[Hahnfeld]

LGTM then

openmp/trunk/runtime/src/include/50/ompt.h.var

macro (omp_state_wait_target_update, 0x082) /* waiting for target update operation */       \
                                                                                                 \
     /* misc (256..511) */                                                                       \
     macro (omp_state_idle, 0x100)           /* waiting for work */                              \
     macro (omp_state_overhead, 0x101)       /* overhead excluding wait states */                \
                                                                                                 \
     /* implementation-specific states (512..) */
 
 
-#define FOREACH_OMPT_MUTEX_IMPL(macro)                                                \
-    macro (ompt_mutex_impl_unknown, 0)      /* unknown implementatin */               \
-    macro (ompt_mutex_impl_spin, 1)         /* based on spin */                       \
-    macro (ompt_mutex_impl_queuing, 2)      /* based on some fair policy */           \
-    macro (ompt_mutex_impl_speculative, 3)  /* based on HW-supported speculation */
+#define FOREACH_KMP_MUTEX_IMPL(macro)                                                \
+    macro (ompt_mutex_impl_unknown, 0)     /* unknown implementation */              \
+    macro (kmp_mutex_impl_spin, 1)         /* based on spin */                       \
+    macro (kmp_mutex_impl_queuing, 2)      /* based on some fair policy */           \
+    macro (kmp_mutex_impl_speculative, 3)  /* based on HW-supported speculation */
 
 #define FOREACH_OMPT_EVENT(macro)                                                                                        \
                                                                                                                          \
     /*--- Mandatory Events ---*/                                                                                         \
     macro (ompt_callback_thread_begin,      ompt_callback_thread_begin_t,       1) /* thread begin                    */ \
     macro (ompt_callback_thread_end,        ompt_callback_thread_end_t,         2) /* thread end                      */ \
                                                                                                                          \
     macro (ompt_callback_parallel_begin,    ompt_callback_parallel_begin_t,     3) /* parallel begin                  */ \
typedef enum ompt_set_result_t {
     ompt_set_sometimes_paired = 3,
     ompt_set_always = 4
 } ompt_set_result_t;
 
 
 /*----------------------
  * mutex implementations
  *----------------------*/
-typedef enum ompt_mutex_impl_t {
-#define ompt_mutex_impl_macro(impl, code) impl = code,
-    FOREACH_OMPT_MUTEX_IMPL(ompt_mutex_impl_macro)
-#undef ompt_mutex_impl_macro
-} ompt_mutex_impl_t;
+typedef enum kmp_mutex_impl_t {
+#define kmp_mutex_impl_macro(impl, code) impl = code,
+    FOREACH_KMP_MUTEX_IMPL(kmp_mutex_impl_macro)
+#undef kmp_mutex_impl_macro
+} kmp_mutex_impl_t;
 
 
 /*****************************************************************************
  * callback signatures
  *****************************************************************************/
 
 /* initialization */
 typedef void (*ompt_interface_fn_t)(void);

openmp/trunk/runtime/src/kmp_atomic.h

 // Atomic locks can easily become contended, so we use queuing locks for them.
 typedef kmp_queuing_lock_t kmp_atomic_lock_t;
 
 static inline void __kmp_acquire_atomic_lock(kmp_atomic_lock_t *lck,
                                              kmp_int32 gtid) {
 #if OMPT_SUPPORT && OMPT_OPTIONAL
   if (ompt_enabled.ompt_callback_mutex_acquire) {
     ompt_callbacks.ompt_callback(ompt_callback_mutex_acquire)(
-        ompt_mutex_atomic, 0, ompt_mutex_impl_queuing, (ompt_wait_id_t)lck,
+        ompt_mutex_atomic, 0, kmp_mutex_impl_queuing, (ompt_wait_id_t)lck,
         OMPT_GET_RETURN_ADDRESS(0));
   }
 #endif
 
   __kmp_acquire_queuing_lock(lck, gtid);
 
 #if OMPT_SUPPORT && OMPT_OPTIONAL
   if (ompt_enabled.ompt_callback_mutex_acquired) {

openmp/trunk/runtime/src/kmp_csupport.cpp

if (ompt_enabled.enabled) {
     /* OMPT state update */
     th->th.ompt_thread_info.wait_id = lck;
     th->th.ompt_thread_info.state = omp_state_wait_ordered;
 
     /* OMPT event callback */
     codeptr_ra = OMPT_LOAD_RETURN_ADDRESS(gtid);
     if (ompt_enabled.ompt_callback_mutex_acquire) {
       ompt_callbacks.ompt_callback(ompt_callback_mutex_acquire)(
-          ompt_mutex_ordered, omp_lock_hint_none, ompt_mutex_impl_spin,
+          ompt_mutex_ordered, omp_lock_hint_none, kmp_mutex_impl_spin,
           (ompt_wait_id_t)lck, codeptr_ra);
     }
   }
 #endif
 
   if (th->th.th_dispatch->th_deo_fcn != 0)
     (*th->th.th_dispatch->th_deo_fcn)(&gtid, &cid, loc);
   else
#endif
   // HLE lock for speculation
   if (hint & omp_lock_hint_speculative)
     return KMP_TSX_LOCK(hle);
 
   return __kmp_user_lock_seq;
 }
 
 #if OMPT_SUPPORT && OMPT_OPTIONAL
-static ompt_mutex_impl_t
+static kmp_mutex_impl_t
 __ompt_get_mutex_impl_type(void *user_lock, kmp_indirect_lock_t *ilock = 0) {
   if (user_lock) {
     switch (KMP_EXTRACT_D_TAG(user_lock)) {
     case 0:
       break;
 #if KMP_USE_FUTEX
     case locktag_futex:
-      return ompt_mutex_impl_queuing;
+      return kmp_mutex_impl_queuing;
 #endif
     case locktag_tas:
-      return ompt_mutex_impl_spin;
+      return kmp_mutex_impl_spin;
 #if KMP_USE_TSX
     case locktag_hle:
-      return ompt_mutex_impl_speculative;
+      return kmp_mutex_impl_speculative;
 #endif
     default:
-      return ompt_mutex_impl_unknown;
+      return kmp_mutex_impl_unknown;
     }
     ilock = KMP_LOOKUP_I_LOCK(user_lock);
   }
   KMP_ASSERT(ilock);
   switch (ilock->type) {
 #if KMP_USE_TSX
   case locktag_adaptive:
   case locktag_rtm:
-    return ompt_mutex_impl_speculative;
+    return kmp_mutex_impl_speculative;
 #endif
   case locktag_nested_tas:
-    return ompt_mutex_impl_spin;
+    return kmp_mutex_impl_spin;
 #if KMP_USE_FUTEX
   case locktag_nested_futex:
 #endif
   case locktag_ticket:
   case locktag_queuing:
   case locktag_drdpa:
   case locktag_nested_ticket:
   case locktag_nested_queuing:
   case locktag_nested_drdpa:
-    return ompt_mutex_impl_queuing;
+    return kmp_mutex_impl_queuing;
   default:
-    return ompt_mutex_impl_unknown;
+    return kmp_mutex_impl_unknown;
   }
 }
 
 // For locks without dynamic binding
-static ompt_mutex_impl_t __ompt_get_mutex_impl_type() {
+static kmp_mutex_impl_t __ompt_get_mutex_impl_type() {
   switch (__kmp_user_lock_kind) {
   case lk_tas:
-    return ompt_mutex_impl_spin;
+    return kmp_mutex_impl_spin;
 #if KMP_USE_FUTEX
   case lk_futex:
 #endif
   case lk_ticket:
   case lk_queuing:
   case lk_drdpa:
-    return ompt_mutex_impl_queuing;
+    return kmp_mutex_impl_queuing;
 #if KMP_USE_TSX
   case lk_hle:
   case lk_rtm:
   case lk_adaptive:
-    return ompt_mutex_impl_speculative;
+    return kmp_mutex_impl_speculative;
 #endif
   default:
-    return ompt_mutex_impl_unknown;
+    return kmp_mutex_impl_unknown;
   }
 }
 #endif
 
 /*!
 @ingroup WORK_SHARING
 @param loc  source location information.
 @param global_tid  global thread number.

openmp/trunk/runtime/src/ompt-general.cpp

 
 typedef struct {
   const char *state_name;
   omp_state_t state_id;
 } omp_state_info_t;
 
 typedef struct {
   const char *name;
-  ompt_mutex_impl_t id;
-} ompt_mutex_impl_info_t;
+  kmp_mutex_impl_t id;
+} kmp_mutex_impl_info_t;
 
 enum tool_setting_e {
   omp_tool_error,
   omp_tool_unset,
   omp_tool_disabled,
   omp_tool_enabled
 };
 
 /*****************************************************************************
  * global variables
  ****************************************************************************/
 
 ompt_callbacks_active_t ompt_enabled;
 
 omp_state_info_t omp_state_info[] = {
 #define omp_state_macro(state, code) {#state, state},
     FOREACH_OMP_STATE(omp_state_macro)
 #undef omp_state_macro
 };
 
-ompt_mutex_impl_info_t ompt_mutex_impl_info[] = {
-#define ompt_mutex_impl_macro(name, id) {#name, name},
-    FOREACH_OMPT_MUTEX_IMPL(ompt_mutex_impl_macro)
-#undef ompt_mutex_impl_macro
+kmp_mutex_impl_info_t kmp_mutex_impl_info[] = {
+#define kmp_mutex_impl_macro(name, id) {#name, name},
+    FOREACH_KMP_MUTEX_IMPL(kmp_mutex_impl_macro)
+#undef kmp_mutex_impl_macro
 };
 
 ompt_callbacks_internal_t ompt_callbacks;
 
 static ompt_start_tool_result_t *ompt_start_tool_result = NULL;
 
 /*****************************************************************************
  * forward declarations
OMPT_API_ROUTINE int ompt_enumerate_states(int current_state, int *next_state,
 
   return 0;
 }
 
 OMPT_API_ROUTINE int ompt_enumerate_mutex_impls(int current_impl,
                                                 int *next_impl,
                                                 const char **next_impl_name) {
   const static int len =
-      sizeof(ompt_mutex_impl_info) / sizeof(ompt_mutex_impl_info_t);
+      sizeof(kmp_mutex_impl_info) / sizeof(kmp_mutex_impl_info_t);
   int i = 0;
   for (i = 0; i < len - 1; i++) {
-    if (ompt_mutex_impl_info[i].id != current_impl)
+    if (kmp_mutex_impl_info[i].id != current_impl)
       continue;
-    *next_impl = ompt_mutex_impl_info[i + 1].id;
-    *next_impl_name = ompt_mutex_impl_info[i + 1].name;
+    *next_impl = kmp_mutex_impl_info[i + 1].id;
+    *next_impl_name = kmp_mutex_impl_info[i + 1].name;
     return 1;
   }
   return 0;
 }
 
 /*****************************************************************************
  * callbacks
  ****************************************************************************/