On Fri, Nov 11, 2016 at 11:43:06AM +0300, Alexander Monakov wrote:
> > > +void
> > > +gomp_barrier_wait_last (gomp_barrier_t *bar)
> > > +{
> > > +#if 0
> > > +  gomp_barrier_state_t state = gomp_barrier_wait_start (bar);
> > > +  if (state & BAR_WAS_LAST)
> > > +    gomp_barrier_wait_end (bar, state);
> > > +#else
> > > +  gomp_barrier_wait (bar);
> > > +#endif
> > > +}
> > 
> > ~~~
> > Any plans to change that later, or shall the #if 0 stuff be just removed?
> 
> Yes, I want to develop a better understanding of bar.h interface contracts and
> see if it's possible to come up with something better suited on PTX.  So I 
> left
> the #if 0 to serve as a reminder what the original code is doing.

Can you then turn it into a comment instead?  There doesn't need to be code,
it can just say that config/linux/bar.c (gomp_barrier_wait_last) has a
separate implementation for the last and that we should consider
reimplementing it similarly.
> 
> > > +/* NVPTX is an accelerator-only target, so this should never be called.  
> > > */
> > > +
> > > +bool
> > > +gomp_target_task_fn (void *data)
> > > +{
> > > +  __builtin_unreachable ();
> > > +}
> > 
> > ~~~
> > Not sure if we don't want to gomp_fatal instead or something similarly
> > loud.
> 
> Originally that was intentional, to allow GCC to DCE paths leading to
> gomp_target_task_fn in PTX libgomp. But then I realized that that idea
> doesn't work fully because such paths may contain calls to other functions,
> and since those functions might not return, the whole path cannot be 
> eliminated.
> Only the part between the preceding non-pure/const call and 
> gomp_target_task_fn
> can be DCE'd.
> 
> > On a related topic, it might be useful to #ifdef out parts of task.c
> > - gomp_target_task_completion, GOMP_PLUGIN_target_task_completion,
> > gomp_create_target_task for nvptx libgomp.a - the first one should be
> > stubbed, the rest left out.  And perhaps at least for now simplify the
> > task priority stuff, as OMP_MAX_TASK_PRIORITY var will not be present
> > on the offloading side anyway.  Can be done incrementally.
> 
> I'm assuming this should use a new macro LIBGOMP_ACCEL_ONLY (name ok?) to cut
> off extraneous code paths?

I'd go for LIBGOMP_OFFLOADED_ONLY, we don't really use accel/ACCEL in libgomp.

        Jakub

Reply via email to