Re: [PATCH 0/10] OpenACC 2.0 support for libgomp

[Julian Brown <julian at codesourcery dot com>]

Hi,

On Wed, 24 Sep 2014 14:32:31 +0200
Jakub Jelinek <jakub@redhat.com> wrote:

> On Tue, Sep 23, 2014 at 07:17:25PM +0100, Julian Brown wrote:
> > The upcoming patch series constitutes our current (still
> > in-progress) implementation of run-time support for OpenACC 2.0 in
> > libgomp. We've tried to build on top of the (also currently WIP)
> > support for OpenMP 4.0's "target" construct, sharing code where
> > possible: because of this, I've also prepared versions of (a fairly
> > minimal, hopefully correct set of) prerequisite patches that apply
> > to current mainline (and were previously on the gomp 4.0 branch),
> > although in many cases we weren't the original authors of those.
> > 
> > Other parts of the OpenACC support for GCC are being sent upstream
> > concurrently with this runtime support (and are co-dependent with
> > it), so unfortunately, though the main part of the implementation
> > (part 7/10) works on our internal branch, I haven't yet been able
> > to convincingly test the series I'm about to post upstream. However
> > this code will be useful to others who are posting their bits of
> > OpenACC support upstream, so perhaps it'd be useful to commit it
> > anyway (we have to start somewhere!).
> 
> Just random comments about all the 10 patches:

Thanks for your comments -- I'm planning to address the things you've
bought up, but will probably change tack a little and do that work on
the gomp-4_0-branch (rather than working directly on mainline). That
way I can (hopefully) send incremental patches rather than working
entirely locally then sending another over-sized patch.

> Cache the return value?  Also, I must say I'm not particularly excited
> about different plugins not supporting both OpenMP 4.0 and OpenACC 2.0
> offloading.  Why is that needed?

For now, because OpenACC supports some stuff that (AFAIK!) OpenMP
doesn't, such as asynchronous execution. The eventual plan is for the
plugin interface to be generic, but we're not there yet.

> +      /* Make sure all the CUDA functions are there if any of them
> are.  */
> +      if (optional_present && optional_present != optional_total)
> +       {
> +         err = "plugin missing OpenACC CUDA handler function";
> +         goto out;
> +       }
> 
> So, any plugin that doesn't support CUDA will not support OpenACC?
> I hoped OpenACC would not be so tied to one particular HW...

The intention was for that section to allow zero CUDA handling
functions, or all of them. For better or worse, OpenACC defines a few
APIs which are target-specific (for NVidia, AMD, Intel so far, IIRC).
An OpenACC application doesn't have to use any of those, of course.

> that is not how ChangeLog entries should look like, if a line is not
> starting with ( after the tab, it should not contain extra spaces
> after the tab, so move Use these. and hack. (and in other spots)
> two columns to the left.

That was merely a copy/paste error of some sort, apologies.

Thanks,

Julian