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

[Kevin Buettner <kevinb at redhat dot com>]

On Tue, 31 Oct 2017 08:03:22 +0100
Jakub Jelinek <jakub@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.

> > It also adds a field named "parent" which is initialized to the thread
> > which created the thread in question.  For non-nested parallel
> > regions, this is always the master thread.  
> 
> What do you need it for and why isn't the current way of querying
> parent (see e.g. omp_get_ancestor_thread_num or omp_get_team_size)
> sufficient for the debugger?  Even if gomp_team_state doesn't contain
> pthread_id, perhaps it would be more space and performance efficient
> to store some pointer into struct gomp_team, gomp_team_state/gomp_thread
> structs are in TLS which should be kept as small as possible.
> Why do you care about which thread called pthread_create, rather than
> what actually owns it right now (is the master thread)?

The cases that I'm considering are variants of this example:

#include <stdio.h>
#include <omp.h>

int
main (int artc, char **argv)
{
  int i = 42;
  int x = 10;
  omp_set_nested (1);
  omp_set_dynamic (0);
#pragma omp parallel num_threads (2) firstprivate(x)
  {
    int j = 43;
    int y = 20;
    x += omp_get_thread_num ();
#pragma omp parallel num_threads (2) firstprivate(y)
    {
      y += omp_get_thread_num ();
      #pragma omp critical
      printf ("inner threads: x=%d, y=%d\n", x, y);
    }
    #pragma omp critical
    printf ("outer threads: x=%d, y=%d, j=%d\n", x, y, j);
  }
  printf ("i = %d\n", i);
}

For this example, neither i nor j appear in the innermost parallel
region.

The variable i will be found on the stack of the master thread.

j, however, is a bit more interesting.  If the GDB user sets a
breakpoint on the "inner threads..." printf, j will either be found on
the on the stack of the master thread or on the stack of the first
thread spawned by the master thread.

So, in order for the debugger to find j, it should first see if it can
be found in the current thread (under consideration).  Next, it should
check the stack of the parent, and then the parent's parent, etc.

I've looked at the implementation of omp_get_ancestor_thread_num(),
but do not see a way to map the value returned to a gomp_thread. 
(Actually, it might be made to work for this example, but if we add
one more level of nesting, we'll need to find a parent thread which
does not appear in the thread pool.) However, I admit that there's
much that I don't understand about libgomp, so it's possible that I've
missed something.  I'd appreciate a pointer on how the existing
mechanisms might be used if that's the case.

(I should also note that in my testing of the above code, it can't work
at the moment anyway since j is optimized out.  I'm assuming that
this can be fixed so that it's not a moot point.)

Kevin