From patchwork Wed Jul 27 15:17:49 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 653399 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3rzzCD2lzLz9t2S for ; Thu, 28 Jul 2016 01:18:15 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=S73tO+Bm; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=AyIXmRVgYeAvbUAc Akvw1ZM8IlAfAfgnpBEvSZNpzf2PPTiqt81gsVwNUD2610lOXXboTZwPcf5Epq+1 QuGpOf5CWCnPrNumOaKck7JcrFN4T1lzhbdzxbkqQLvpqnG9OyU49Pcj3cFeFhsW glQMsHKxpyQMsV9/tsAJ87dg+Yw= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version:content-type :content-transfer-encoding; s=default; bh=PrxpXRxY0gQFiQaC9RN5Xv rT/Rw=; b=S73tO+BmiyH8ijB1AAgs9I8Pq/7Xa6adEa37QaFF7dr01K8Bg9r3lq h/xNek8eBZr7SpSM4M0qw+/uXI2ZynSahZq3TuUktFSLdV69ti4zOXhfGOCbvv2N c1ahs4D81Y8ROmSNrNqo58+Q99FY7iBKPMHIR+/sYpnEpWNdD4ols= Received: (qmail 56548 invoked by alias); 27 Jul 2016 15:18:07 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 56537 invoked by uid 89); 27 Jul 2016 15:18:06 -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=sk:c_parse, visually, quality 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 27 Jul 2016 15:17:56 +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 1bSQam-0002jk-V4 from Thomas_Schwinge@mentor.com for gcc-patches@gcc.gnu.org; Wed, 27 Jul 2016 08:17:53 -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; Wed, 27 Jul 2016 16:17:51 +0100 From: Thomas Schwinge To: Subject: Use correct location information for OpenACC shape and simple clauses in C/C++ User-Agent: Notmuch/0.9-101-g81dad07 (http://notmuchmail.org) Emacs/24.4.1 (x86_64-pc-linux-gnu) Date: Wed, 27 Jul 2016 17:17:49 +0200 Message-ID: <87twfbjejm.fsf@hertz.schwinge.homeip.net> MIME-Version: 1.0 Hi! I found that for a lot of OpenACC (and potentially also OpenMP) clauses (in C/C++ front ends; didn't look at Fortran), we use wrong location information. The problem is that c_parser_oacc_all_clauses/c_parser_omp_all_clauses calls cp_parser_omp_clause_name to determine the pragma_omp_clause c_kind, and that function (as documented) consumes the clause token before returning. So, when we then do "c_parser_peek_token (parser)->location" or similar in some clause parsing function, that will return the location information of the token _after_ the clause token, which -- at least very often -- is not desirable, in particular if that location information is used then in a build_omp_clause call, which should point to the clause token itself, and not whatever follows after that. Probably that all went unnoticed for so long, because the GCC testsuite largely is running with -fno-diagnostics-show-caret, so we don't visually see the wrong location information (and nobody pays attention to the colum information as given, for example, as line 2, column 32 in "[...]/c-c++-common/goacc/routine-2.c:2:32: error: [...]". There seems to be a lot of inconsistency in that in all the clause parsing; here is just a first patch to fix the immediate problem I've been observing. OK for trunk already, or need to clean this all up in one go? Do we need this on release branches, as a "quality of implementation" fix (wrong diagnostic locations)? commit bac4c04ca1d52c56a3583f5958e116c62b889d5a Author: Thomas Schwinge Date: Wed Jul 27 16:55:56 2016 +0200 Use correct location information for OpenACC shape and simple clauses in C/C++ gcc/c/ * c-parser.c (c_parser_oacc_shape_clause) (c_parser_oacc_simple_clause): Add loc formal parameter. Adjust all users. gcc/cp/ * parser.c (cp_parser_oacc_shape_clause): Add loc formal parameter. Adjust all users. --- gcc/c/c-parser.c | 25 +++++++++++++------------ gcc/cp/parser.c | 12 +++++++----- 2 files changed, 20 insertions(+), 17 deletions(-) Grüße Thomas diff --git gcc/c/c-parser.c gcc/c/c-parser.c index 0031481..82ac855 100644 --- gcc/c/c-parser.c +++ gcc/c/c-parser.c @@ -11623,12 +11623,12 @@ c_parser_omp_clause_num_workers (c_parser *parser, tree list) */ static tree -c_parser_oacc_shape_clause (c_parser *parser, omp_clause_code kind, +c_parser_oacc_shape_clause (c_parser *parser, location_t loc, + omp_clause_code kind, const char *str, tree list) { const char *id = "num"; tree ops[2] = { NULL_TREE, NULL_TREE }, c; - location_t loc = c_parser_peek_token (parser)->location; if (kind == OMP_CLAUSE_VECTOR) id = "length"; @@ -11758,12 +11758,12 @@ c_parser_oacc_shape_clause (c_parser *parser, omp_clause_code kind, seq */ static tree -c_parser_oacc_simple_clause (c_parser *parser, enum omp_clause_code code, - tree list) +c_parser_oacc_simple_clause (c_parser * /* parser */, location_t loc, + enum omp_clause_code code, tree list) { check_no_duplicate_clause (list, code, omp_clause_code_name[code]); - tree c = build_omp_clause (c_parser_peek_token (parser)->location, code); + tree c = build_omp_clause (loc, code); OMP_CLAUSE_CHAIN (c) = list; return c; @@ -13089,7 +13089,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, c_name = "async"; break; case PRAGMA_OACC_CLAUSE_AUTO: - clauses = c_parser_oacc_simple_clause (parser, OMP_CLAUSE_AUTO, + clauses = c_parser_oacc_simple_clause (parser, here, OMP_CLAUSE_AUTO, clauses); c_name = "auto"; break; @@ -13139,7 +13139,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, break; case PRAGMA_OACC_CLAUSE_GANG: c_name = "gang"; - clauses = c_parser_oacc_shape_clause (parser, OMP_CLAUSE_GANG, + clauses = c_parser_oacc_shape_clause (parser, here, OMP_CLAUSE_GANG, c_name, clauses); break; case PRAGMA_OACC_CLAUSE_HOST: @@ -13151,8 +13151,9 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, c_name = "if"; break; case PRAGMA_OACC_CLAUSE_INDEPENDENT: - clauses = c_parser_oacc_simple_clause (parser, OMP_CLAUSE_INDEPENDENT, - clauses); + clauses = c_parser_oacc_simple_clause (parser, here, + OMP_CLAUSE_INDEPENDENT, + clauses); c_name = "independent"; break; case PRAGMA_OACC_CLAUSE_LINK: @@ -13200,7 +13201,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, c_name = "self"; break; case PRAGMA_OACC_CLAUSE_SEQ: - clauses = c_parser_oacc_simple_clause (parser, OMP_CLAUSE_SEQ, + clauses = c_parser_oacc_simple_clause (parser, here, OMP_CLAUSE_SEQ, clauses); c_name = "seq"; break; @@ -13214,7 +13215,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, break; case PRAGMA_OACC_CLAUSE_VECTOR: c_name = "vector"; - clauses = c_parser_oacc_shape_clause (parser, OMP_CLAUSE_VECTOR, + clauses = c_parser_oacc_shape_clause (parser, here, OMP_CLAUSE_VECTOR, c_name, clauses); break; case PRAGMA_OACC_CLAUSE_VECTOR_LENGTH: @@ -13227,7 +13228,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, break; case PRAGMA_OACC_CLAUSE_WORKER: c_name = "worker"; - clauses = c_parser_oacc_shape_clause (parser, OMP_CLAUSE_WORKER, + clauses = c_parser_oacc_shape_clause (parser, here, OMP_CLAUSE_WORKER, c_name, clauses); break; default: diff --git gcc/cp/parser.c gcc/cp/parser.c index 2797ec4..b78c3de 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -30372,13 +30372,13 @@ cp_parser_oacc_single_int_clause (cp_parser *parser, omp_clause_code code, */ static tree -cp_parser_oacc_shape_clause (cp_parser *parser, omp_clause_code kind, +cp_parser_oacc_shape_clause (cp_parser *parser, location_t loc, + omp_clause_code kind, const char *str, tree list) { const char *id = "num"; cp_lexer *lexer = parser->lexer; tree ops[2] = { NULL_TREE, NULL_TREE }, c; - location_t loc = cp_lexer_peek_token (lexer)->location; if (kind == OMP_CLAUSE_VECTOR) id = "length"; @@ -32248,7 +32248,7 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask, break; case PRAGMA_OACC_CLAUSE_GANG: c_name = "gang"; - clauses = cp_parser_oacc_shape_clause (parser, OMP_CLAUSE_GANG, + clauses = cp_parser_oacc_shape_clause (parser, here, OMP_CLAUSE_GANG, c_name, clauses); break; case PRAGMA_OACC_CLAUSE_HOST: @@ -32330,7 +32330,8 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask, break; case PRAGMA_OACC_CLAUSE_VECTOR: c_name = "vector"; - clauses = cp_parser_oacc_shape_clause (parser, OMP_CLAUSE_VECTOR, + clauses = cp_parser_oacc_shape_clause (parser, here, + OMP_CLAUSE_VECTOR, c_name, clauses); break; case PRAGMA_OACC_CLAUSE_VECTOR_LENGTH: @@ -32345,7 +32346,8 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask, break; case PRAGMA_OACC_CLAUSE_WORKER: c_name = "worker"; - clauses = cp_parser_oacc_shape_clause (parser, OMP_CLAUSE_WORKER, + clauses = cp_parser_oacc_shape_clause (parser, here, + OMP_CLAUSE_WORKER, c_name, clauses); break; default: