Bug fixes in OMPT support

[jmellorcrummey]

Summary

1. in kmp_csupport.c, move computation of parameters only needed for OMPT tracing inside a conditional to reduce overhead if not receiving ompt_event_master_begin callbacks.
2. in kmp_gsupport.c
   • remove spurious reset of OMPT reenter_runtime_frame (which is set in its caller, GOMP_parallel_start
   • correct placement of #if OMP_TRACE so that state is maintained even if tracing support not included.
3. in z_Linux_util.c
   • add architecture independent support for OMPT by setting and resetting OMPT's exit_frame_ptr before and after invoking a microtask.
4. On the Intel MIC, the loader refuses to retain static symbols in the libomp.so shared library, even though tools need them. The loader could not be bullied into doing so. To accommodate this, I changed the visibility of OMPT placeholder functions to public. This required additions in exports.so.txt, adding extern "C" scoping in ompt-general.c so that the public placeholder symbols won't be mangled.

[jcownie]

It looks generally fine, but I am nervous about putting the ompt state placeholders in OpenMP's namespace (by giving them thename prefix "omp_" ). Is there a reason not to name them "ompt_", which would move them into our namespace?

[jmellorcrummey]

Moved OMPT placeholders from omp namespace to ompt namespace.

[jlpeyton]

LGTM as well. In the future, can you provide more context to the patch. Details can be seen here: llvm.org/docs/Phabricator.html

runtime/src/exports_so.txt

VERSION {
     global: # Exported symbols.
 
         #
         # "Normal" symbols.
         #
         omp_*;     # Standard OpenMP functions.
         ompt_initialize;     # OMPT initialization interface
         ompt_control;        # OMPT control interface
+        
+        #
+        # OMPT state placeholders
+        #
+        omp_idle;
+        omp_overhead;
+        omp_barrier_wait;
+        omp_task_wait;
+        omp_mutex_wait;
+
         ompc_*;    # omp.h renames some standard functions to ompc_*.
         kmp_*;     # Intel extensions.
         kmpc_*;    # Intel extensions.
         __kmpc_*;  # Functions called by compiler-generated code.
         GOMP_*;    # GNU C compatibility functions.
 
         _You_must_link_with_*;     # Mutual detection/MS compatibility symbols.
 

runtime/src/kmp_csupport.c

__kmpc_master(ident_t *loc, kmp_int32 global_tid)
     if( ! TCR_4( __kmp_init_parallel ) )
         __kmp_parallel_initialize();
 
     if( KMP_MASTER_GTID( global_tid ))
         status = 1;
 
 #if OMPT_SUPPORT && OMPT_TRACE
     if (status) {
-        kmp_info_t  *this_thr        = __kmp_threads[ global_tid ];
-        kmp_team_t  *team            = this_thr -> th.th_team;
         if ((ompt_status == ompt_status_track_callback) &&
             ompt_callbacks.ompt_callback(ompt_event_master_begin)) {
+            kmp_info_t  *this_thr        = __kmp_threads[ global_tid ];
+            kmp_team_t  *team            = this_thr -> th.th_team;
+
             int  tid = __kmp_tid_from_gtid( global_tid );
             ompt_callbacks.ompt_callback(ompt_event_master_begin)(
                 team->t.ompt_team_info.parallel_id,
                 team->t.t_implicit_task_taskdata[tid].ompt_task_info.task_id);
         }
     }
 #endif
 

runtime/src/kmp_gsupport.c

__kmp_GOMP_fork_call(ident_t *loc, int gtid, void (*unwrapped_task)(void *), microtask_t wrapper, int argc,...)
     int rc;
     kmp_info_t *thr = __kmp_threads[gtid];
     kmp_team_t *team = thr->th.th_team;
     int tid = __kmp_tid_from_gtid(gtid);
 
     va_list ap;
     va_start(ap, argc);
 
-#if OMPT_SUPPORT
-    team->t.t_implicit_task_taskdata[tid].
-        ompt_task_info.frame.reenter_runtime_frame = NULL;
-#endif
-
     rc = __kmp_fork_call(loc, gtid, fork_context_gnu, argc,
 #if OMPT_SUPPORT
       VOLATILE_CAST(void *) unwrapped_task,
 #endif
       wrapper, __kmp_invoke_task_func,
 #if (KMP_ARCH_X86_64 || KMP_ARCH_ARM || KMP_ARCH_AARCH64) && KMP_OS_LINUX
       &ap
 #else
       ap
 #endif
       );
 
     va_end(ap);
 
     if (rc) {
         __kmp_run_before_invoked_task(gtid, tid, thr, team);
     }
 
-#if OMPT_SUPPORT && OMPT_TRACE
+#if OMPT_SUPPORT 
     if (ompt_status & ompt_status_track) {
+#if OMPT_TRACE
         ompt_team_info_t *team_info = __ompt_get_teaminfo(0, NULL);
         ompt_task_info_t *task_info = __ompt_get_taskinfo(0);
 
         // implicit task callback
         if ((ompt_status == ompt_status_track_callback) &&
             ompt_callbacks.ompt_callback(ompt_event_implicit_task_begin)) {
             ompt_callbacks.ompt_callback(ompt_event_implicit_task_begin)(
                 team_info->parallel_id, task_info->task_id);
         }
+#endif
         thr->th.ompt_thread_info.state = ompt_state_work_parallel;
     }
 #endif
 }
 
 static void
 __kmp_GOMP_serialized_parallel(ident_t *loc, kmp_int32 gtid, void (*task)(void *))
 {

runtime/src/ompt-general.c

OMPT_API_ROUTINE void *ompt_get_task_function(int depth)
     return __ompt_get_task_function_internal(depth);
 }
 
 
 /*****************************************************************************
  * placeholders
  ****************************************************************************/
 
+// Don't define this as static. The loader may choose to eliminate the symbol
+// even though it is needed by tools.  
+#define OMPT_API_PLACEHOLDER 
 
-OMPT_API_ROUTINE void omp_idle(void)
+// Ensure that placeholders don't have mangled names in the symbol table.
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+
+OMPT_API_PLACEHOLDER void omp_idle(void)  
 {
-    // this function is a placeholder used to represent the calling context of
+    // This function is a placeholder used to represent the calling context of
     // idle OpenMP worker threads. It is not meant to be invoked.
     assert(0);
 }
 
 
-OMPT_API_ROUTINE void omp_overhead(void)
+OMPT_API_PLACEHOLDER void omp_overhead(void)
 {
-    // this function is a placeholder used to represent the OpenMP context of
+    // This function is a placeholder used to represent the OpenMP context of
     // threads working in the OpenMP runtime.  It is not meant to be invoked.
     assert(0);
 }
 
 
-OMPT_API_ROUTINE void omp_barrier_wait(void)
+OMPT_API_PLACEHOLDER void omp_barrier_wait(void)
 {
-    // this function is a placeholder used to represent the OpenMP context of
+    // This function is a placeholder used to represent the OpenMP context of
     // threads waiting for a barrier in the OpenMP runtime. It is not meant
     // to be invoked.
     assert(0);
 }
 
 
-OMPT_API_ROUTINE void omp_task_wait(void)
+OMPT_API_PLACEHOLDER void omp_task_wait(void)
 {
-    // this function is a placeholder used to represent the OpenMP context of
+    // This function is a placeholder used to represent the OpenMP context of
     // threads waiting for a task in the OpenMP runtime. It is not meant
     // to be invoked.
     assert(0);
 }
 
 
-OMPT_API_ROUTINE void omp_mutex_wait(void)
+OMPT_API_PLACEHOLDER void omp_mutex_wait(void)
 {
-    // this function is a placeholder used to represent the OpenMP context of
+    // This function is a placeholder used to represent the OpenMP context of
     // threads waiting for a mutex in the OpenMP runtime. It is not meant
     // to be invoked.
     assert(0);
 }
 
+#ifdef __cplusplus
+};
+#endif
+
 
 /*****************************************************************************
  * compatability
  ****************************************************************************/
 
 OMPT_API_ROUTINE int ompt_get_ompt_version()
 {
     return OMPT_VERSION;

runtime/src/z_Linux_util.c

 # endif // KMP_OS_DARWIN
 
 #endif // USE_LOAD_BALANCE
 
 
 #if KMP_COMPILER_GCC && !(KMP_ARCH_X86 || KMP_ARCH_X86_64 || KMP_ARCH_PPC64 || KMP_ARCH_AARCH64)
 
 int __kmp_invoke_microtask( microtask_t pkfn, int gtid, int tid, int argc,
-        void *p_argv[] )
+        void *p_argv[] 
+#if OMPT_SUPPORT
+        , void **exit_frame_ptr
+#endif
+)
 {
     int argc_full = argc + 2;
     int i;
     ffi_cif cif;
     ffi_type *types[argc_full];
     void *args[argc_full];
     void *idp[2];
 
+#if OMPT_SUPPORT
+    *exit_frame_ptr = __builtin_frame_address(0);
+#endif
     /* We're only passing pointers to the target. */
     for (i = 0; i < argc_full; i++)
         types[i] = &ffi_type_pointer;
 
     /* Ugly double-indirection, but that's how it goes... */
     idp[0] = &gtid;
     idp[1] = &tid;
     args[0] = &idp[0];
     args[1] = &idp[1];
 
     for (i = 0; i < argc; i++)
         args[2 + i] = &p_argv[i];
 
     if (ffi_prep_cif(&cif, FFI_DEFAULT_ABI, argc_full,
                 &ffi_type_void, types) != FFI_OK)
         abort();
 
     ffi_call(&cif, (void (*)(void))pkfn, NULL, args);
 
+#if OMPT_SUPPORT
+    *exit_frame_ptr = 0;
+#endif
+
     return 1;
 }
 
 #endif // KMP_COMPILER_GCC && !(KMP_ARCH_X86 || KMP_ARCH_X86_64 || KMP_ARCH_PPC64)
 
 #if KMP_ARCH_PPC64 || KMP_ARCH_AARCH64
 
 // we really only need the case with 1 argument, because CLANG always build
 // a struct of pointers to shared variables referenced in the outlined function
 int
 __kmp_invoke_microtask( microtask_t pkfn,
                         int gtid, int tid,
-                        int argc, void *p_argv[] ) {
+                        int argc, void *p_argv[] 
+#if OMPT_SUPPORT
+                        , void **exit_frame_ptr
+#endif
+) 
+{
+#if OMPT_SUPPORT
+  *exit_frame_ptr = __builtin_frame_address(0);
+#endif
+
   switch (argc) {
   default:
     fprintf(stderr, "Too many args to microtask: %d!\n", argc);
     fflush(stderr);
     exit(-1);
   case 0:
     (*pkfn)(&gtid, &tid);
     break;
case 14:
     break;
   case 15:
     (*pkfn)(&gtid, &tid, p_argv[0], p_argv[1], p_argv[2], p_argv[3], p_argv[4],
             p_argv[5], p_argv[6], p_argv[7], p_argv[8], p_argv[9], p_argv[10],
             p_argv[11], p_argv[12], p_argv[13], p_argv[14]);
     break;
   }
 
+#if OMPT_SUPPORT
+  *exit_frame_ptr = 0;
+#endif
+
   return 1;
 }
 
 #endif
 
 // end of file //