From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 97783 invoked by alias); 17 Jun 2016 14:34:47 -0000 Mailing-List: contact fortran-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: fortran-owner@gcc.gnu.org Received: (qmail 97765 invoked by uid 89); 17 Jun 2016 14:34:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 17 Jun 2016 14:34:45 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 27493C04B305; Fri, 17 Jun 2016 14:34:44 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-51.ams2.redhat.com [10.36.116.51]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u5HEYg0N028157 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 17 Jun 2016 10:34:43 -0400 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id u5HEYeUc005501; Fri, 17 Jun 2016 16:34:41 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id u5HEYcW1005500; Fri, 17 Jun 2016 16:34:38 +0200 Date: Fri, 17 Jun 2016 14:34:00 -0000 From: Jakub Jelinek To: Cesar Philippidis Cc: gcc-patches@gcc.gnu.org, Fortran List Subject: Re: OpenACC wait clause Message-ID: <20160617143438.GB7387@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <20160607111307.GQ7387@tucnak.redhat.com> <5756E1B6.605@codesourcery.com> <20160607150252.GS7387@tucnak.redhat.com> <57636CF5.40400@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57636CF5.40400@codesourcery.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-SW-Source: 2016-06/txt/msg00045.txt.bz2 On Thu, Jun 16, 2016 at 08:22:29PM -0700, Cesar Philippidis wrote: > --- 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) I think it is still problematic. Most of the parsing fortran FE errors are deferred, meaning that if you don't reject the whole gfc_match_omp_clauses, then no diagnostics is actually emitted. Both gfc_match (" ( %e )", &c->async_expr) and match_oacc_expr_list (" (", &c->wait_list, false) IMHO can return MATCH_YES, MATCH_NO and MATCH_ERROR, and I believe you need to do different actions in each case. In particular, if something is optional, then for MATCH_YES you should accept it (continue) and not set needs_space, because after ) you don't need space. If MATCH_NO, then you should accept it too (because it is optional), and set needs_space = true; first and perhaps do whatever else you need to do. If MATCH_ERROR, then you should make sure not to accept it, e.g. by doing break; or making sure continue will not be done (which one depends on whether it might be validly parsed as some other clause, which is very likely not the case). In the above changes, you do it all except for the MATCH_ERROR handling, where you still do continue; and thus I bet diagnostics for it won't be reported. E.g. for !$omp acc parallel async(&abc) !$omp acc end parallel end no diagnostics is reported. Looking around, there are many more issues like that, e.g. match_oacc_clause_gang(c) (note, wrong formatting) also ignores MATCH_ERROR, etc. > @@ -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) > { Can you explain this change? I bet it again suffers from the above mentioned issue. If match_oacc_expr_list returns MATCH_YES, I believe you want false, false, true as you don't need space in between the closing ) of the wait_list and name of next clause. Note, does OpenACC allow also comma in that case? !$acc wait (whatever),async ? If match_oacc_expr_list returns MATCH_NO, then IMHO it should be true, true, true, because you don't want to accept !$acc waitasync and also don't want to accept !$acc wait,async And if match_oacc_expr_list returns MATCH_ERROR, you should reject it, so that diagnostics is emitted. Jakub