On Wed, 21 Oct 2015, Jakub Jelinek wrote:
> > -#if defined HAVE_TLS || defined USE_EMUTLS
> > +#if defined __nvptx__
> > +extern struct gomp_thread *nvptx_thrs;
> 
> What kind of address space is this variable?  It should be
> a per-CTA var, so that different teams have different, and
> simultaneous target regions have different too.

As written it's in global accelerator memory.  Indeed it's broken with
simultaneous target regions, and to unbreak that I'd like to place it in
shared memory (but that would require expanding address-space support
a bit more, exposing shared-memory space to C source code).

> I'm surprised that for team.c you chose to adjust the shared source,
> rather than copy and remove all the cruft you don't need/want.
> 
> That includes the LIBGOMP_USE_PTHREADS guarded parts, all the thread binding
> stuff etc.  I'd like to see at least for comparison how much actually
> remained in there.

Diffstat for the copy/remove patch is 66+/474-, almost all of removed 470
lines are in gomp_team_start, which counts only ~150 lines after removals.

> > @@ -88,7 +138,8 @@ gomp_thread_start (void *xdata)
> >    thr->task = data->task;
> >    thr->place = data->place;
> >  
> > -  thr->ts.team->ordered_release[thr->ts.team_id] = &thr->release;
> > +  if (thr->ts.team)
> > +    thr->ts.team->ordered_release[thr->ts.team_id] = &thr->release;
> 
> Why this?  Lots of gomp_thread_start other places assume thr->ts.team is
> non-NULL.  And, this isn't even guarded with __nvptx__, we don't want
> to slow down host thread start.

thr->ts.team is NULL when entering from gomp_nvptx_main, and should be setup
by master thread from gomp_team_start while we are waiting on first
threads_dock barrier in gomp_thread_start (currently I don't expect
data->nested to be true on nvptx).

Should I drop the "if" and instead simply guard the statement with #ifndef
__nvptx__?

Thanks.
Alexander

Reply via email to