public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Cesar Philippidis <cesar@codesourcery.com>
To: Jakub Jelinek <jakub@redhat.com>
Cc: <gcc-patches@gcc.gnu.org>, Fortran List <fortran@gcc.gnu.org>
Subject: Re: OpenACC wait clause
Date: Fri, 17 Jun 2016 03:22:00 -0000	[thread overview]
Message-ID: <57636CF5.40400@codesourcery.com> (raw)
In-Reply-To: <20160607150252.GS7387@tucnak.redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]

On 06/07/2016 08:02 AM, Jakub Jelinek wrote:
> On Tue, Jun 07, 2016 at 08:01:10AM -0700, Cesar Philippidis wrote:
>> On 06/07/2016 04:13 AM, Jakub Jelinek wrote:
>>
>>> I've noticed
>>>           if ((mask & OMP_CLAUSE_WAIT)
>>>               && !c->wait
>>>               && gfc_match ("wait") == MATCH_YES)
>>>             {
>>>               c->wait = true; 
>>>               match_oacc_expr_list (" (", &c->wait_list, false);
>>>               continue;
>>>             }
>>> which looks just weird and confusing.  Why isn't this instead:
>>>           if ((mask & OMP_CLAUSE_WAIT)
>>>               && !c->wait
>>> 	      && (match_oacc_expr_list ("wait (", &c->wait_list, false)
>>> 		  == MATCH_YES))
>>>             {
>>>               c->wait = true; 
>>>               continue;
>>>             }
>>> ?  Otherwise you happily accept wait without following (, perhaps even
>>> combined with another clause without any space in between etc.
>>
>> Both acc wait and async accept optional parenthesis arguments. E.g.,
>>
>>   #pragma acc wait
>>
>> blocks for all of the async streams to complete before proceeding, whereas
>>
>>   #pragma acc wait (1, 5)
>>
>> only blocks for async streams 1 and 5.
> 
> But then you need to set need_space = true; if it doesn't have the ( after
> it.

I was distracted with acc routine stuff, so this took me a little longer
to get around to this. In addition to that problem with the wait clause,
I discovered a similar problem with the async clause and the wait
directive. It this patch ok for trunk and gcc-6?

Cesar


[-- Attachment #2: fortran-wait-async.diff --]
[-- Type: text/x-patch, Size: 3287 bytes --]

2016-06-16  Cesar Philippidis  <cesar@codesourcery.com>

	gcc/fortran/
	* openmp.c (gfc_match_omp_clauses): Update use a needs_space for the
	OpenACC wait and async clauses.
	(gfc_match_oacc_wait): Ensure that there is a space when the optional
	parenthesis is missing.

	gcc/testsuite/
	* gfortran.dg/goacc/asyncwait-2.f95: Add additional test coverage.
	* gfortran.dg/goacc/asyncwait-4.f95: Likewise.

diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 2c92794..435c709 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -677,7 +677,6 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, uint64_t mask,
 	      && gfc_match ("async") == MATCH_YES)
 	    {
 	      c->async = true;
-	      needs_space = false;
 	      if (gfc_match (" ( %e )", &c->async_expr) != MATCH_YES)
 		{
 		  c->async_expr
@@ -685,6 +684,7 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, uint64_t mask,
 					     gfc_default_integer_kind,
 					     &gfc_current_locus);
 		  mpz_set_si (c->async_expr->value.integer, GOMP_ASYNC_NOVAL);
+		  needs_space = true;
 		}
 	      continue;
 	    }
@@ -1328,7 +1328,8 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, uint64_t mask,
 	      && gfc_match ("wait") == MATCH_YES)
 	    {
 	      c->wait = true;
-	      match_oacc_expr_list (" (", &c->wait_list, false);
+	      if (match_oacc_expr_list (" (", &c->wait_list, false) == MATCH_NO)
+		needs_space = true;
 	      continue;
 	    }
 	  if ((mask & OMP_CLAUSE_WORKER)
@@ -1649,7 +1650,7 @@ gfc_match_oacc_wait (void)
   gfc_expr_list *wait_list = NULL, *el;
 
   match_oacc_expr_list (" (", &wait_list, true);
-  gfc_match_omp_clauses (&c, OACC_WAIT_CLAUSES, false, false, true);
+  gfc_match_omp_clauses (&c, OACC_WAIT_CLAUSES, false, true, true);
 
   if (gfc_match_omp_eos () != MATCH_YES)
     {
diff --git a/gcc/testsuite/gfortran.dg/goacc/asyncwait-2.f95 b/gcc/testsuite/gfortran.dg/goacc/asyncwait-2.f95
index db0ce1f..7b2ae07 100644
--- a/gcc/testsuite/gfortran.dg/goacc/asyncwait-2.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/asyncwait-2.f95
@@ -83,6 +83,18 @@ program asyncwait
   end do
   !$acc end parallel ! { dg-error "Unexpected \\\!\\\$ACC END PARALLEL" }
 
+  !$acc parallel copyin (a(1:N)) copy (b(1:N)) waitasync ! { dg-error "Unclassifiable OpenACC directive" }
+  do i = 1, N
+     b(i) = a(i)
+  end do
+  !$acc end parallel ! { dg-error "Unexpected \\\!\\\$ACC END PARALLEL" }
+
+  !$acc parallel copyin (a(1:N)) copy (b(1:N)) asyncwait ! { dg-error "Unclassifiable OpenACC directive" }
+  do i = 1, N
+     b(i) = a(i)
+  end do
+  !$acc end parallel ! { dg-error "Unexpected \\\!\\\$ACC END PARALLEL" }
+  
   !$acc parallel copyin (a(1:N)) copy (b(1:N)) wait
   do i = 1, N
      b(i) = a(i)
diff --git a/gcc/testsuite/gfortran.dg/goacc/asyncwait-4.f95 b/gcc/testsuite/gfortran.dg/goacc/asyncwait-4.f95
index cd64ef3..01349b0 100644
--- a/gcc/testsuite/gfortran.dg/goacc/asyncwait-4.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/asyncwait-4.f95
@@ -34,4 +34,6 @@ program asyncwait
   !$acc wait async (1.0) ! { dg-error "ASYNC clause at \\\(1\\\) requires a scalar INTEGER expression" }
 
   !$acc wait async 1 ! { dg-error "Unexpected junk in \\\!\\\$ACC WAIT at" }
+
+  !$acc waitasync ! { dg-error "Unexpected junk in \\\!\\\$ACC WAIT at" }
 end program asyncwait

       reply	other threads:[~2016-06-17  3:22 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20160607111307.GQ7387@tucnak.redhat.com>
     [not found] ` <5756E1B6.605@codesourcery.com>
     [not found]   ` <20160607150252.GS7387@tucnak.redhat.com>
2016-06-17  3:22     ` Cesar Philippidis [this message]
2016-06-17 14:34       ` Jakub Jelinek
2016-06-24 15:42         ` Cesar Philippidis
2016-06-24 15:53           ` Jakub Jelinek
2016-06-27 18:36             ` Cesar Philippidis
2016-06-27 19:23               ` Jakub Jelinek
2016-06-27 23:22                 ` Cesar Philippidis
2016-06-28  7:08                   ` Jakub Jelinek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57636CF5.40400@codesourcery.com \
    --to=cesar@codesourcery.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).