From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38108 invoked by alias); 2 Dec 2015 23:37:23 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 38098 invoked by uid 89); 2 Dec 2015 23:37:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 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; Wed, 02 Dec 2015 23:37:21 +0000 Received: from svr-orw-fem-06.mgc.mentorg.com ([147.34.97.120]) by relay1.mentorg.com with esmtp id 1a4Gxa-00024Y-2O from Cesar_Philippidis@mentor.com ; Wed, 02 Dec 2015 15:37:18 -0800 Received: from [127.0.0.1] (147.34.91.1) by SVR-ORW-FEM-06.mgc.mentorg.com (147.34.97.120) with Microsoft SMTP Server id 14.3.224.2; Wed, 2 Dec 2015 15:37:17 -0800 Subject: Re: [1/2] OpenACC routine support To: Thomas Schwinge References: <5637B1CF.5060408@acm.org> <5637B7C7.70901@acm.org> <20151103153533.GQ478@tucnak.redhat.com> <56413AF1.8070101@acm.org> <5641808F.10302@codesourcery.com> <20151110081603.GV5675@tucnak.redhat.com> <564CCB29.30909@codesourcery.com> <871tb6nsez.fsf@kepler.schwinge.homeip.net> <565DB379.5070306@codesourcery.com> CC: Nathan Sidwell , GCC Patches , Jakub Jelinek From: Cesar Philippidis Message-ID: <565F80AD.5090107@mentor.com> Date: Wed, 02 Dec 2015 23:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <565DB379.5070306@codesourcery.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-SW-Source: 2015-12/txt/msg00381.txt.bz2 On 12/01/2015 06:49 AM, Cesar Philippidis wrote: > On 12/01/2015 06:40 AM, Thomas Schwinge wrote: > >> I noticed while working on other test cases: >> >> On Wed, 18 Nov 2015 11:02:01 -0800, Cesar Philippidis wrote: >>> --- a/gcc/cp/parser.c >>> +++ b/gcc/cp/parser.c >> >>> @@ -1318,13 +1318,21 @@ cp_finalize_omp_declare_simd (cp_parser *parser, tree fndecl) >>> } >>> } >>> >>> -/* Diagnose if #pragma omp routine isn't followed immediately >>> - by function declaration or definition. */ >>> +/* Diagnose if #pragma acc routine isn't followed immediately by function >>> + declaration or definition. */ >>> >>> static inline void >>> cp_ensure_no_oacc_routine (cp_parser *parser) >>> { >>> - cp_finalize_oacc_routine (parser, NULL_TREE, false, true); >>> + if (parser->oacc_routine && !parser->oacc_routine->error_seen) >>> + { >>> + tree clauses = parser->oacc_routine->clauses; >>> + location_t loc = OMP_CLAUSE_LOCATION (TREE_PURPOSE(clauses)); >>> + >>> + error_at (loc, "%<#pragma oacc routine%> not followed by function " >>> + "declaration or definition"); >>> + parser->oacc_routine = NULL; >>> + } >>> } >> >> "#pragma acc routine", not "oacc". Also in a few other places. > > Good eyes. Thanks for catching that. > >> Next, in the function quoted above, you use "not followed by function >> declaration or definition", but you use "not followed by a single >> function declaration or definition" in a lot of (but not all) other >> places -- is that intentional? > > I probably wasn't being consistent. Which error message do you prefer? > I'll take a look at what the c front end does. > >> For example: >> >>> cp_parser_oacc_routine (cp_parser *parser, cp_token *pragma_tok, >>> enum pragma_context context) >>> { >>> [...] >>> + error_at (OMP_CLAUSE_LOCATION (parser->oacc_routine->clauses), >>> + "%<#pragma oacc routine%> not followed by a single " >>> + "function declaration or definition"); >> >> "a single". >> >>> [...] >>> + if (parser->oacc_routine >>> + && !parser->oacc_routine->error_seen >>> + && !parser->oacc_routine->fndecl_seen) >>> + error_at (loc, "%<#pragma acc routine%> not followed by " >>> + "function declaration or definition"); >> >> Not "a single". >> >>> + >>> + data.tokens.release (); >>> + parser->oacc_routine = NULL; >>> + } >>> + } >>> +} >>> + >>> +/* Finalize #pragma acc routine clauses after direct declarator has >>> + been parsed, and put that into "oacc routine" attribute. */ >> >> There is no "oacc routine" attribute (anymore)? > > You're right, it was renamed to 'oacc function'. > >>> +static tree >>> +cp_parser_late_parsing_oacc_routine (cp_parser *parser, tree attrs) >>> +{ >>> [...] >>> + if ((!data->error_seen && data->fndecl_seen) >>> + || data->tokens.length () != 1) >>> + { >>> + error_at (loc, "%<#pragma oacc routine%> not followed by a single " >>> + "function declaration or definition"); >> >> "a single". >> >> (I have not verified all of the parser(s) source code.) > > Thanks. I'll go through and update the comments and error messages. Here's the updated patch. The test cases were written in a way such that none of them needed to be updated with these changes. I'm tempted to commit this as obvious, but I want to make sure you're ok with these new messages. The major change is to report these errors as "pragma acc routine not followed by a function declaration or definition". I think that's more descriptive then "not followed by a single function". That said, it looks like the c front end uses the latter error message. Is this OK or do you prefer the "not followed by a single function" message? Cesar