Re: OpenACC ICV acc-default-async-var

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi Chung-Lin!

On Mon, 19 Nov 2018 16:33:30 +0900, Chung-Lin Tang <chunglin_tang@mentor.com> wrote:
> Hi Thomas,
> actually the current version of the acc_get/set_default_async patch is
> combined into:
> https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01426.html
> 
> This patch you're referring here was a version from early 2017.

I know, but I intend to handle these changes individually, for these are
separate things.

> I'll try to reply to the still applying comments here below.

Thanks.

> On 2018/11/18 10:36 AM, Thomas Schwinge wrote:
> >> --- include/gomp-constants.h	(revision 245382)
> >> +++ include/gomp-constants.h	(working copy)
> > 
> >>   /* Asynchronous behavior.  Keep in sync with
> >>      libgomp/{openacc.h,openacc.f90,openacc_lib.h}:acc_async_t.  */
> >>   
> >> +#define GOMP_ASYNC_DEFAULT		0
> >>   #define GOMP_ASYNC_NOVAL		-1
> >>   #define GOMP_ASYNC_SYNC			-2
> > 
> > This means that "acc_set_default_async(acc_async_default)" will set
> > acc-default-async-var to "0", that is, the same as
> > "acc_set_default_async(0)".  It thus follows that
> > "async"/"async(acc_async_noval)" is the same as "async(0)".  Is that
> > intentional?
> > 
> > It is in line with the OpenACC 2.5 specification: "initial value [...] is
> > implementation defined", but I wonder why map it to "async(0)", and not
> > to its own, unspecified, but separate queue.  In the latter case,
> > "acc_async_default" etc. would then map to a negative value to denote
> > this unspecified, but separate queue (and your changes would need to be
> > adapted for that).
> > 
> > I have not verified whether we're currently already having (on trunk
> > and/or openacc-gcc-8-branch) the semantics of the queue of
> > "async(acc_async_noval)" mapping to the same queue as "async(0)"?
> 
> As long as the thr->default_async variable == 0 (as it is initially)
> then async(acc_async_noval) maps to async(0).
> 
> > I'm fine to accept your changes as proposed (basically, everthing from
> > your patch posted that has a "default_async" in it), for that's an
> > incremental improvement anyway.  But -- unless you tell me I've
> > misunderstood something -- I'll get the issue I raised clarified with the
> > OpenACC technical committee, and we will then later improve this further.
> > 
> > No matter what the outcome, the implementation-defined behavior should be
> > documented.  (Can do that once we get the intentions clarified.)
> 
> Well, the current submitted implementation of async queues manages an array of them
> for each thread. So the intuitive default queue is the first (index 0), and
> to support reverting to default when accepting 'acc_async_default' as an argument,
> defining acc_async_default == 0 is the logical choice.
> 
> The 'default' async is not symbolically a specific queue, it is simply a thread-local
> variable for what is referred by default when 'acc_async_noval' is encountered.
>  From that sense, initializing it as some negative integer doesn't make sense

That's not my understanding, though, and is not what is currently
implemented in trunk, where "async(acc_async_noval)" ("async" without an
argument) currently is a separate queue, different from all "async(a)"
with "a" nonnegative.

> Of course, if really desired, we implement the "default default" to be an alternative
> queue separate from the non-negative queue space, but I feel this is overkill.

I'm looking into clarifying that, and then adjusting the code
accordingly; shouldn't be too difficult.


Grüße
 Thomas