From patchwork Mon Jul 27 14:58:43 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julian Brown X-Patchwork-Id: 500464 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 111DB1402A5 for ; Tue, 28 Jul 2015 00:59:06 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=buAzOYSk; 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:date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type; q=dns; s=default; b=sUAIkBNxhfmGexo7 QzyDJWvT6JEMrKKNC29CFc96wl9k9mHZcDLqEuceSZMoTDf7ICU/zBQC4lx/sKuf mDtdSs5cJahUUTkiQ17Kfi+Tm5sbCUSgrFYawbkiHsGdmLBNEfB7yNiXNyvAjEFd Y3cGlIN2WtcOSaISMP605OCmcwo= 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:date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type; s=default; bh=2BspkTRquzCD/ohXgpM5zb hCBD8=; b=buAzOYSk5vN6jpqzvOw0UStMDlNY2Ce/8cR5sIftTBNiaIT/cGaAa0 GzldIy3e6ZlngmAfE6aWTeFvZRTzhO5aEEHlDM1K3xsycyhJNHWUBKbUNGXJD6vh /rF8x9HkFgRUUBZ4zJCc6jQIDlvLFTTiu38KIRrLm2rygufMkNh04= Received: (qmail 102438 invoked by alias); 27 Jul 2015 14:58:59 -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 102407 invoked by uid 89); 27 Jul 2015 14:58:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 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; Mon, 27 Jul 2015 14:58:57 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-02.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1ZJjrh-0002MP-4A from Julian_Brown@mentor.com for gcc-patches@gcc.gnu.org; Mon, 27 Jul 2015 07:58:53 -0700 Received: from octopus (137.202.0.76) by SVR-IES-FEM-02.mgc.mentorg.com (137.202.0.106) with Microsoft SMTP Server id 14.3.224.2; Mon, 27 Jul 2015 15:58:51 +0100 Date: Mon, 27 Jul 2015 15:58:43 +0100 From: Julian Brown To: Thomas Schwinge CC: , Cesar Philippidis , Subject: Re: [gomp4] Remove device-specific filtering during parsing for OpenACC Message-ID: <20150727155843.0da2313c@octopus> In-Reply-To: <87mvyvuel1.fsf@kepler.schwinge.homeip.net> References: <20150716163212.777040e9@octopus> <87mvyvuel1.fsf@kepler.schwinge.homeip.net> MIME-Version: 1.0 X-IsSubscribed: yes On Fri, 17 Jul 2015 14:57:14 +0200 Thomas Schwinge wrote: > In combination with the equivant change to > gcc/cp/parser.c:cp_parser_oacc_all_clauses, > gcc/c-family/c-omp.c:c_oacc_filter_device_types, and transitively also > the struct identifier_hasher and c_oacc_extract_device_id function > preceding it, are now unused. (Not an exhaustive list; have not > checked which other auxilliary functions etc. Cesar has added in his > device_type changes.) Does it make any sense to keep these for > later, or dump them now? The attached patch removes this dead code... > > --- a/gcc/c/c-typeck.c > > +++ b/gcc/c/c-typeck.c > > @@ -12568,6 +12568,10 @@ c_finish_omp_clauses (tree clauses, bool > > oacc) pc = &OMP_CLAUSE_CHAIN (c); > > continue; > > > > + case OMP_CLAUSE_DEVICE_TYPE: > > + pc = &OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c); > > + continue; > > + > > case OMP_CLAUSE_INBRANCH: > > case OMP_CLAUSE_NOTINBRANCH: > > if (branch_seen) > > From a quick glance only, this seems to be different from the C++ > front end (have not checked Fortran). > > I have not looked at what the front end parsing is now actually > doing; is it just attaching any clauses following a device_type > clause to the latter? (The same should be done for all front ends, > obviously. Even if it's not important right now, because of the > sorry diagnostic that will be emitted later on as soon as there is > one device_type clause, this should best be addressed now, while you > still remember what's going on here ;-) so that there will be no bad > surprises once we actually implement the handling in OMP > lowering/streaming/device compilers.) > > Do we need manually need to take care to > "finalize" (c_finish_omp_clauses et al.) such "masked" clause chains, > or will the right thing happen automatically? ...and fixes the C and C++ frontend to "finalize" parsed device_type clauses properly (although so far "finalization" doesn't do anything for the clauses that can be associated with a device_type clause anyway, so there's no actual change in behaviour). I haven't moved the "sorry" reporting for the unsupported device_type clause to scan_sharing_clauses because it doesn't seem to be particularly a more logical place, and doing so breaks the tests that scan the omp-low dumps. I will apply to gomp4 branch as obvious, shortly. Thanks, Julian ChangeLog gcc/ * c-family/c-omp.c (c_oacc_extract_device_id, identifier_hasher) (c_oacc_filter_device_types): Remove dead code. * c/c-typeck.c (c_finish_omp_clauses): Add scanning for sub-clauses of device_type clause. * cp/semantics.c (finish_omp_clauses): Likewise. commit e24a9cd14d4b8b5dab8b37218b29844787809648 Author: Julian Brown Date: Mon Jul 27 07:31:10 2015 -0700 Clause finalization cleanups and dead code removal. diff --git a/gcc/c-family/c-omp.c b/gcc/c-family/c-omp.c index b76de69..10190d7 100644 --- a/gcc/c-family/c-omp.c +++ b/gcc/c-family/c-omp.c @@ -1081,132 +1081,6 @@ c_omp_predetermined_sharing (tree decl) return OMP_CLAUSE_DEFAULT_UNSPECIFIED; } -/* Return a numerical code representing the device_type. Currently, - only device_type(nvidia) is supported. All device_type parameters - are treated as case-insensitive keywords. */ - -static int -c_oacc_extract_device_id (const char *device) -{ - if (!strcasecmp (device, "nvidia")) - return GOMP_DEVICE_NVIDIA_PTX; - else if (!strcmp (device, "*")) - return GOMP_DEVICE_DEFAULT; - return GOMP_DEVICE_NONE; -} - -struct identifier_hasher : ggc_cache_ptr_hash -{ - static hashval_t hash (tree t) { return htab_hash_pointer (t); } - static bool equal (tree a, tree b) - { - return !strcmp(IDENTIFIER_POINTER (a), IDENTIFIER_POINTER (b)); - } -}; - -/* Filter out the list of unsupported OpenACC device_types. */ - -tree -c_oacc_filter_device_types (tree clauses) -{ - tree c, prev; - tree dtype = NULL_TREE; - tree seen_nvidia = NULL_TREE; - tree seen_default = NULL_TREE; - hash_table *dt_htab - = hash_table::create_ggc (10); - - /* First scan for all device_type clauses. */ - for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c)) - { - if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEVICE_TYPE) - { - tree t; - - for (t = OMP_CLAUSE_DEVICE_TYPE_DEVICES (c); t; t = TREE_CHAIN (t)) - { - if (dt_htab->find (t)) - { - error_at (OMP_CLAUSE_LOCATION (c), - "duplicate device_type (%s)", - IDENTIFIER_POINTER (t)); - goto filter_dtype; - } - - int code = c_oacc_extract_device_id (IDENTIFIER_POINTER (t)); - - if (code == GOMP_DEVICE_DEFAULT) - seen_default = OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c); - else if (code == GOMP_DEVICE_NVIDIA_PTX) - seen_nvidia = OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c); - else - { - /* The OpenACC technical committee advises compilers - to silently ignore unknown devices. */ - } - - tree *slot = dt_htab->find_slot (t, INSERT); - *slot = t; - } - } - } - - /* Don't do anything if there aren't any device_type clauses. */ - if (dt_htab->elements () == 0) - return clauses; - - if (seen_nvidia) - dtype = seen_nvidia; - else if (seen_default) - dtype = seen_default; - else - goto filter_dtype; - - dtype = seen_nvidia ? seen_nvidia : seen_default; - - /* Now filter out clauses if necessary. */ - for (c = clauses; c && OMP_CLAUSE_CODE (c) != OMP_CLAUSE_DEVICE_TYPE; - c = OMP_CLAUSE_CHAIN (c)) - { - tree t; - - prev = NULL_TREE; - - for (t = dtype; t; t = OMP_CLAUSE_CHAIN (t)) - { - if (OMP_CLAUSE_CODE (t) == OMP_CLAUSE_CODE (c)) - { - /* Remove c from clauses. */ - tree next = OMP_CLAUSE_CHAIN (c); - - if (prev) - OMP_CLAUSE_CHAIN (prev) = next; - - break; - } - } - - prev = c; - } - - filter_dtype: - /* Remove all device_type clauses. Those clauses are located at the - beginning of the clause list. */ - for (c = clauses; c && OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEVICE_TYPE; - c = OMP_CLAUSE_CHAIN (c)) - ; - - if (c == NULL_TREE) - return dtype; - - clauses = c; - for (prev = c, c = OMP_CLAUSE_CHAIN (c); c; c = OMP_CLAUSE_CHAIN (c)) - prev = c; - - OMP_CLAUSE_CHAIN (prev) = dtype; - return clauses; -} - /* Split the 'clauses' into a set of 'loop' clauses and a set of 'not-loop' clauses. */ diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index dcc246c..36d027e 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -12569,7 +12569,9 @@ c_finish_omp_clauses (tree clauses, bool oacc) continue; case OMP_CLAUSE_DEVICE_TYPE: - pc = &OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c); + OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c) + = c_finish_omp_clauses (OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c), oacc); + pc = &OMP_CLAUSE_CHAIN (c); continue; case OMP_CLAUSE_INBRANCH: diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 1ce1dfa..c1983d4 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -5951,9 +5951,14 @@ finish_omp_clauses (tree clauses, bool oacc) case OMP_CLAUSE_BIND: case OMP_CLAUSE_NOHOST: case OMP_CLAUSE_TILE: - case OMP_CLAUSE_DEVICE_TYPE: break; + case OMP_CLAUSE_DEVICE_TYPE: + OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c) + = finish_omp_clauses (OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c), oacc); + pc = &OMP_CLAUSE_CHAIN (c); + continue; + case OMP_CLAUSE_INBRANCH: case OMP_CLAUSE_NOTINBRANCH: if (branch_seen)