Hi Chung-Lin!
On Mon, 19 Nov 2018 16:33:30 +0900, Chung-Lin Tang <chunglin_t...@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
