Re: [PATCH] Add fields to struct gomp_thread for debugging purposes

[Kevin Buettner]

On Sat, 4 Nov 2017 21:39:14 -0700
Kevin Buettner <kev...@redhat.com> wrote:

> On Tue, 31 Oct 2017 08:03:22 +0100
> Jakub Jelinek <ja...@redhat.com> wrote:
> 
> > On Mon, Oct 30, 2017 at 04:06:15PM -0700, Kevin Buettner wrote:  
> > > This patch adds a new member named "pthread_id" to the gomp_thread
> > > struct.  It is initialized in team.c.    
> > 
> > That part is reasonable, though it is unclear how the debugger will
> > query it (through OMPD, or through hardcoded name lookup of the struct and
> > field in libgomp's debug info, something else).  But the field certainly
> > has to be guarded by #ifdef LIBGOMP_USE_PTHREADS, otherwise it will break
> > NVPTX offloading or any other pthread-less libgomp ports.
> > Another question is exact placement of the field, struct gomp_thread
> > vs. struct gomp_team_state etc.  Maybe it is ok, as the pthread_id is
> > the same once the thread is created, doesn't change when we create more
> > levels.  
> 
> Assuming we can figure out how to work the rest of it out, I'll submit
> a new patch with the appropriate ifdef.

I've decided to try for a more incremental approach.  This patch,
below, retains the portion that looked reasonable to you.  I've
stripped out the portions for finding the thread parent and have
added the ifdef guards that you asked for.

Is this part okay?

___

Add field to struct gomp_thread for debugging purposes

This patch adds a new member named "pthread_id" to the gomp_thread
struct.  It is initialized in team.c.

This new field serves no purpose in a normally running OpenMP
program.  It is intended to be used by a debugger for identifying
threads.

I've done a "make bootstrap" and have regression tested these changes
with no regressions found.

libgomp/ChangeLog:
    
        * libgomp.h (struct gomp_thread): Add new member "pthread_id".
        * team.c (gomp_thread_start): Set pthread_id.
        (gomp_team_start): Initialize master thread.
---
 libgomp/libgomp.h |  6 ++++++
 libgomp/team.c    | 13 +++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 940b5b8..5dafb3c 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -611,6 +611,12 @@ struct gomp_thread
      to any place.  */
   unsigned int place;
 
+#ifdef LIBGOMP_USE_PTHREADS
+  /* The pthread id associated with this thread.  This is required for
+     debugging.  */
+  pthread_t pthread_id;
+#endif
+
   /* User pthread thread pool */
   struct gomp_thread_pool *thread_pool;
 };
diff --git a/libgomp/team.c b/libgomp/team.c
index 676614a..6292b22 100644
--- a/libgomp/team.c
+++ b/libgomp/team.c
@@ -92,6 +92,10 @@ gomp_thread_start (void *xdata)
 
   thr->ts.team->ordered_release[thr->ts.team_id] = &thr->release;
 
+#ifdef LIBGOMP_USE_PTHREADS
+  thr->pthread_id = pthread_self ();
+#endif
+
   /* Make thread pool local. */
   pool = thr->thread_pool;
 
@@ -718,6 +722,15 @@ gomp_team_start (void (*fn) (void *), void *data, unsigned 
nthreads,
       attr = &thread_attr;
     }
 
+  /* Add the master thread to threads[] and record its pthread id too.  */
+  if (pool->threads[0] == NULL)
+    {
+      pool->threads[0] = thr;
+#ifdef LIBGOMP_USE_PTHREADS
+      thr->pthread_id = pthread_self ();
+#endif
+    }
+
   start_data = gomp_alloca (sizeof (struct gomp_thread_start_data)
                            * (nthreads-i));