On Wed, Nov 22, 2017 at 08:25:35AM -0700, Kevin Buettner wrote:
> 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?
Actually, isn't this redundant information at least on Linux?
How would the debugger find the pthread_id field in the TLS?
If it is TLS and libgomp uses -ftls-model=initial-exec on Linux, then
the difference between the base of the TLS area (which is I believe
pthread_self ()) and struct gomp_thread is fixed (and can be found in the
debug info on most linux targets, except e.g. aarch64 which doesn't have
as/ld support for dtoff-ish relocations). So, if the debugger can find
the gomp_thread, then it can find pthread_self () and vice versa. Dunno
if libthread_db or infinity notes in libpthread provide any info for this,
and/or infinity notes in libgomp could add some further info so that the
debugger can do this portably.
Of course, for targets where this isn't possible I'm not against your patch
if it is conditionalized on those targets that can't do better.
In any case, IMHO the debugger shouldn't hardcode that stuff but
ask infinity notes in libgomp and libpthread how to compute it.
Or as temporary alternative to the infinity notes it could be even a
function in the library that given struct gomp_thread returns corresponding
pthread_t. Though, it would be nice to know exactly what the debugger wants
to do. Perhaps it is also already part of OMPD and we could just implement
subset of it.
Jakub
