libgomp: Add master thread to thread pool

For debugging purposes, I need to be able to find the master thread
in the thread pool.

Without this patch, I see over 20 failures in the tests that I've
written for GDB.

I've also tested this in the gcc tree - no regressions.

libgomp/ChangeLog:

	* team.c (gomp_team_start): Initialize pool->threads[0].
---
 libgomp/team.c | 6 ++++++
 1 file changed, 6 insertions(+)

Ping.

On Fri, 22 Feb 2019 18:11:44 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> For debugging purposes, I need to be able to find the master thread
> in the thread pool.
> 
> Without this patch, I see over 20 failures in the tests that I've
> written for GDB.
> 
> I've also tested this in the gcc tree - no regressions.
> 
> libgomp/ChangeLog:
> 
> 	* team.c (gomp_team_start): Initialize pool->threads[0].
> ---
>  libgomp/team.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libgomp/team.c b/libgomp/team.c
> index 2b2e9750da5..e331c9b72c0 100644
> --- a/libgomp/team.c
> +++ b/libgomp/team.c
> @@ -477,11 +477,17 @@ gomp_team_start (void (*fn) (void *), void *data, unsigned nthreads,
>  	 make no effort to expand gomp_threads_size geometrically.  */
>        if (nthreads >= pool->threads_size)
>  	{
> +	  bool need_init = (pool->threads == NULL);
>  	  pool->threads_size = nthreads + 1;
>  	  pool->threads
>  	    = gomp_realloc (pool->threads,
>  			    pool->threads_size
>  			    * sizeof (struct gomp_thread *));
> +	  if (need_init)
> +	    {
> +	      /* Add current (master) thread to threads[].  */
> +	      pool->threads[0] = thr;
> +	    }
>  	}
>  
>        /* Release existing idle threads.  */
>

On Fri, Feb 22, 2019 at 06:11:44PM -0700, Kevin Buettner wrote:
> For debugging purposes, I need to be able to find the master thread
> in the thread pool.
> 
> Without this patch, I see over 20 failures in the tests that I've
> written for GDB.
> 
> I've also tested this in the gcc tree - no regressions.
> 
> libgomp/ChangeLog:
> 
> 	* team.c (gomp_team_start): Initialize pool->threads[0].

If it is really needed, then it might be much better to just do it
unconditionally, i.e.
	  pool->threads[0] = thr;
after the realloc, I don't see what doing it conditionally buys us,
gomp_realloc is already slow enough and even from cache-line ownership POV
at least when it is actually reallocated it shouldn't make a difference.
That said, I'd like to better understand what do you need it for.

When do you need to find the master thread, do you have some special case
when number of threads in the team is 1?  Because in that case pool->threads
will be often NULL.  I admit handling that case is quite trivial, but
I'm still asking if it is handled.

What do you do for the case of nested parallelism?  There are no thread
pools ATM, so how do you find the master thread in that case?

In theory, albeit more costly, one could find the thread that owns the
thread pool just by iterating over all threads and checking which thread
has gomp_tls_data.thread_pool equal to the pool in which you'd query
pool->threads, though I admit it is likely unacceptable e.g. for
ompd_get_thread_in_parallel we'll need to implement at some point too
(finding a master thread is passing 0 as the thread num in that case).

	Jakub

On Mon, 25 Mar 2019 19:30:57 +0100
Jakub Jelinek <jakub@redhat.com> wrote:

> On Fri, Feb 22, 2019 at 06:11:44PM -0700, Kevin Buettner wrote:
> > For debugging purposes, I need to be able to find the master thread
> > in the thread pool.
> > 
> > Without this patch, I see over 20 failures in the tests that I've
> > written for GDB.
> > 
> > I've also tested this in the gcc tree - no regressions.
> > 
> > libgomp/ChangeLog:
> > 
> > 	* team.c (gomp_team_start): Initialize pool->threads[0].  
> 
> If it is really needed, then it might be much better to just do it
> unconditionally, i.e.
> 	  pool->threads[0] = thr;
> after the realloc, I don't see what doing it conditionally buys us,
> gomp_realloc is already slow enough and even from cache-line ownership POV
> at least when it is actually reallocated it shouldn't make a difference.

That's fine with me.

> That said, I'd like to better understand what do you need it for.

I need to be able to find a thread's parent thread.  Given a thread T, I
want to find a thread P which is the "parent" of thread T.  By parent
thread, I mean the one which spawned the child thread.

For the non-nested parallelism case, the master thread is the parent
of each of the other top level threads.

Finding the parent thread for nested (non top level) threads is harder.
I don't currently have an acceptable solution for this case.  (See
below for a link to my earlier solution.)

> When do you need to find the master thread, do you have some special case
> when number of threads in the team is 1?  Because in that case pool->threads
> will be often NULL.  I admit handling that case is quite trivial, but
> I'm still asking if it is handled.

Yes, the case where the number of threads is (just) 1 is handled by
the code that I've written.

> What do you do for the case of nested parallelism?  There are no thread
> pools ATM, so how do you find the master thread in that case?

I presented a solution for finding the thread parent of a non top level
thread a while back, but that solution wasn't acceptable to you at that
time.  See:

https://gcc.gnu.org/ml/gcc-patches/2017-10/msg02259.html

I have not yet come up with a better approach.  I'm still trying to get the
rest of my work to land.  I plan to look at it again once I'm further
along.  I'll note however, that the code which I've written for GDB
does attempt to follow the complete ancestor chain.  The missing
piece of the puzzle is being able to find the information I need from
libgomp.

Here is a simpler patch which sets the master thread unconditionally:

libgomp/ChangeLog:

	* team.c (gomp_team_start): Initialize pool->threads[0].

diff --git a/libgomp/team.c b/libgomp/team.c
index 2b2e9750da5..c422da3701d 100644
--- a/libgomp/team.c
+++ b/libgomp/team.c
@@ -482,6 +482,8 @@ gomp_team_start (void (*fn) (void *), void *data, unsigned nthreads,
 	    = gomp_realloc (pool->threads,
 			    pool->threads_size
 			    * sizeof (struct gomp_thread *));
+	  /* Add current (master) thread to threads[].  */
+	  pool->threads[0] = thr;
 	}
 
       /* Release existing idle threads.  */

On Tue, Mar 26, 2019 at 03:42:09PM -0700, Kevin Buettner wrote:
> libgomp/ChangeLog:
> 
> 	* team.c (gomp_team_start): Initialize pool->threads[0].
> 
> diff --git a/libgomp/team.c b/libgomp/team.c
> index 2b2e9750da5..c422da3701d 100644
> --- a/libgomp/team.c
> +++ b/libgomp/team.c
> @@ -482,6 +482,8 @@ gomp_team_start (void (*fn) (void *), void *data, unsigned nthreads,
>  	    = gomp_realloc (pool->threads,
>  			    pool->threads_size
>  			    * sizeof (struct gomp_thread *));
> +	  /* Add current (master) thread to threads[].  */
> +	  pool->threads[0] = thr;
>  	}
>  
>        /* Release existing idle threads.  */

Ok for trunk, let's deal with nested threads in a follow-up.

	Jakub