From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 86981 invoked by alias); 16 Aug 2016 22:17:02 -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 86958 invoked by uid 89); 16 Aug 2016 22:17:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=reconsidered X-Spam-User: qpsmtpd, 2 recipients X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 16 Aug 2016 22:16:51 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1bZmf9-0004Qs-MT from Thomas_Schwinge@mentor.com ; Tue, 16 Aug 2016 15:16:48 -0700 Received: from hertz.schwinge.homeip.net (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Tue, 16 Aug 2016 23:16:46 +0100 From: Thomas Schwinge To: Cesar Philippidis CC: "gcc-patches@gcc.gnu.org" , Fortran List , Tobias Burnus , Jakub Jelinek Subject: Re: [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling In-Reply-To: <001e7a79-d2d8-fa63-2b88-20a346f3b4a7@codesourcery.com> References: <579973CB.3070006@codesourcery.com> <579AD9C9.3030804@codesourcery.com> <5776D55A.4030002@codesourcery.com> <878tw35o6k.fsf@kepler.schwinge.homeip.net> <20160811154026.GV14857@tucnak.redhat.com> <871t1vi851.fsf@hertz.schwinge.homeip.net> <001e7a79-d2d8-fa63-2b88-20a346f3b4a7@codesourcery.com> User-Agent: Notmuch/0.9-101-g81dad07 (http://notmuchmail.org) Emacs/24.4.1 (x86_64-pc-linux-gnu) Date: Tue, 16 Aug 2016 22:17:00 -0000 Message-ID: <87fuq4gy0m.fsf@hertz.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-SW-Source: 2016-08/txt/msg00074.txt.bz2 Hi! On Mon, 15 Aug 2016 18:54:49 -0700, Cesar Philippidis wrote: > For the moment, I'm ignoring the > device_type problem and handling all of the matching errors in > gfc_match_oacc_routine. OK for the moment; my idea has been to do it generally enough already now, using generic infrastructure I have been/will be adding for C/C++, so that device_type support will later be simple to implement for all three front ends. But, let's leave that aside for the moment. > You're patch was handling those errors in > add_attributes_to_decls, which I think is too late. I can't tell why that's "too late". Anyway, we can save this discussion for later. ;-) > Thomas, does this patch ok to you for gomp4? Yes, please commit, so that we can move this whole thing forward. :-) A few quick comments anyway: > --- a/gcc/fortran/openmp.c > +++ b/gcc/fortran/openmp.c > @@ -1993,19 +2002,24 @@ gfc_match_oacc_routine (void) > dims =3D gfc_oacc_routine_dims (c); > if (dims =3D=3D OACC_FUNCTION_NONE) > { > gfc_error ("Multiple loop axes specified in !$ACC ROUTINE at %C"); > - goto cleanup; > + > + /* Don't abort early, because it's important to let the user > + know of any potential duplicate routine directives. */ > + seen_error =3D true; > } Hmm, I don't know if that's really important? I mean, if we run into "Multiple loop axes specified", that is a hard semantic error already? Anyway, this can be reconsidered later. > if (isym !=3D NULL) > { > if (c && (c->gang || c->worker || c->vector)) > { > - gfc_error ("Intrinsic function specified in !$ACC ROUTINE ( NAME )" > - " at %C, with incompatible GANG, WORKER, or VECTOR clause"); > + gfc_error ("Intrinsic symbol specified in !$ACC ROUTINE ( NAME ) " > + "at %C, with incompatible clauses specifying the level " > + "of parallelism"); > goto cleanup; > } You're re-introducing the wording I had used earlier, before I changed that to the more specific one mentioning the clause names. Why change that again? Also something the can be reconsidered later. (Goes together with the gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-2.f changes.) > --- a/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f > +++ b/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f > @@ -1,17 +1,13 @@ > -! Check for valid clauses with intrinsic function specified in !$ACC ROU= TINE ( NAME ). > - > SUBROUTINE sub_1 > IMPLICIT NONE > -!$ACC ROUTINE (ABORT) > -!$ACC ROUTINE (ABORT) SEQ > +!$ACC ROUTINE (ABORT) SEQ VECTOR ! { dg-error "Intrinsic symbol specifie= d in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses = specifying the level of parallelism" } >=20=20 > CALL ABORT > END SUBROUTINE sub_1 >=20=20 > MODULE m_w_1 > IMPLICIT NONE > -!$ACC ROUTINE (ABORT) SEQ > -!$ACC ROUTINE (ABORT) > +!$ACC ROUTINE (ABORT) VECTOR GANG ! { dg-error "Intrinsic symbol specifi= ed in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses= specifying the level of parallelism" } This changes the intention of this test file? Another thing that can be reconsidered later. So, please commit as-is, and I'll then base my other changes on top of that. Gr=C3=BC=C3=9Fe Thomas