From patchwork Wed Oct 28 11:00:55 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 537309 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 15EBB140D24 for ; Wed, 28 Oct 2015 22:01:52 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=VZXn9sZK; 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:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; q=dns; s=default; b=IABCVlFD/C9yJBpz PbAAqXwotMWQSt9R+OQG+6A1ihg2zt5TDXJqfnnTOFNTlxcMq2ZxGDYUPKQqGlJ3 w8+ScYK+ZreQe+RrQtgHAeVfehky3J5PfHfKYrGfV4/6MXi6p/pVA634s66yP8QD GadcKgrB3A1n80OE1yZEoaSO+e4= 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:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; s=default; bh=5dskE7Qpddjyw4I0+GnPig fOv+E=; b=VZXn9sZKVIFo9atOv9uvG9bdCq072lmlUQYM92czf5ZDvQDs/JJjza BGpRYYtu1cIVToycTt/MiN74BhwioXB6xFvKhZXBleAogM27jPJEoiKY57fXnfGn sYB9DS3CFf270t1/9C7sArwFwK3oblIU+BD+dmi3WDFyTdA/Zi50c= Received: (qmail 104784 invoked by alias); 28 Oct 2015 11:01:35 -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 104684 invoked by uid 89); 28 Oct 2015 11:01:34 -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, 28 Oct 2015 11:01:27 +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 1ZrOTr-0002Sf-7t from Thomas_Schwinge@mentor.com for gcc-patches@gcc.gnu.org; Wed, 28 Oct 2015 04:01:23 -0700 Received: from feldtkeller.schwinge.homeip.net (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; Wed, 28 Oct 2015 11:01:21 +0000 From: Thomas Schwinge To: Cesar Philippidis , "gcc-patches@gcc.gnu.org" CC: james norris Subject: Re: [gomp4] fortran cleanups and c/c++ loop parsing backport In-Reply-To: <562FC41A.4030308@codesourcery.com> References: <562FC41A.4030308@codesourcery.com> User-Agent: Notmuch/0.9-125-g4686d11 (http://notmuchmail.org) Emacs/24.5.1 (i586-pc-linux-gnu) Date: Wed, 28 Oct 2015 12:00:55 +0100 Message-ID: <87vb9r45qw.fsf@kepler.schwinge.homeip.net> MIME-Version: 1.0 Hi Cesar! On Tue, 27 Oct 2015 11:36:10 -0700, Cesar Philippidis wrote: > This patch contains the following: > > * C front end changes from trunk: > https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02528.html > > * C++ front end changes from trunk: > https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02540.html > > * Proposed fortran cleanups and enhanced error reporting changes: > https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02288.html I suppose the latter is a prerequisite for other Fortran front end changes you've also committed? Otherwise, why not get that patch into trunk first? That sould save me from having to deal with potentially more merge conflicts later on... > In addition, I've also added a couple of more test cases and updated the > way that combined directives are handled in fortran. Because the > device_type clauses form a chain of gfc_omp_clauses, I couldn't reuse > gfc_split_omp_clauses for combined parallel and kernels loops. So that's > why I introduced a new gfc_filter_oacc_combined_clauses function. Thanks, but... > I'll apply this patch to gomp-4_0-branch shortly. I know that I should > have broken this patch down into smaller patches Yes. > but it was already > arranged as one big patch in my source tree. You're using Git, so that's not a good excuse. :-P > --- a/gcc/fortran/trans-openmp.c > +++ b/gcc/fortran/trans-openmp.c > @@ -3634,12 +3634,65 @@ gfc_trans_omp_do (gfc_code *code, gfc_exec_op op, stmtblock_t *pblock, > return gfc_finish_block (&block); > } > > -/* parallel loop and kernels loop. */ > +/* Helper function to filter combined oacc constructs. ORIG_CLAUSES > + contains the unfiltered list of clauses. LOOP_CLAUSES corresponds to > + the filter list of loop clauses corresponding to the enclosed list. > + This function is called recursively to handle device_type clauses. */ > + > +static void > +gfc_filter_oacc_combined_clauses (gfc_omp_clauses **orig_clauses, > + gfc_omp_clauses **loop_clauses) > +{ > + if (*orig_clauses == NULL) > + { > + *loop_clauses = NULL; > + return; > + } > + > + *loop_clauses = gfc_get_omp_clauses (); > + > + memset (*loop_clauses, 0, sizeof (gfc_omp_clauses)); This has already been created zero-initialized. > + (*loop_clauses)->gang = (*orig_clauses)->gang; > + (*orig_clauses)->gang = false; > + (*loop_clauses)->gang_expr = (*orig_clauses)->gang_expr; > + (*orig_clauses)->gang_expr = NULL; > + (*loop_clauses)->gang_static = (*orig_clauses)->gang_static; > + (*orig_clauses)->gang_static = false; > + (*loop_clauses)->vector = (*orig_clauses)->vector; > + (*orig_clauses)->vector = false; > + (*loop_clauses)->vector_expr = (*orig_clauses)->vector_expr; > + (*orig_clauses)->vector_expr = NULL; > + (*loop_clauses)->worker = (*orig_clauses)->worker; > + (*orig_clauses)->worker = false; > + (*loop_clauses)->worker_expr = (*orig_clauses)->worker_expr; > + (*orig_clauses)->worker_expr = NULL; > + (*loop_clauses)->seq = (*orig_clauses)->seq; > + (*orig_clauses)->seq = false; > + (*loop_clauses)->independent = (*orig_clauses)->independent; > + (*orig_clauses)->independent = false; > + (*loop_clauses)->par_auto = (*orig_clauses)->par_auto; > + (*orig_clauses)->par_auto = false; > + (*loop_clauses)->acc_collapse = (*orig_clauses)->acc_collapse; > + (*orig_clauses)->acc_collapse = false; > + (*loop_clauses)->collapse = (*orig_clauses)->collapse; > + /* Don't reset (*orig_clauses)->collapse. */ Why? (Extend source code comment?) The original code (cited just below) did this differently. > + (*loop_clauses)->tile = (*orig_clauses)->tile; > + (*orig_clauses)->tile = false; > + (*loop_clauses)->tile_list = (*orig_clauses)->tile_list; > + (*orig_clauses)->tile_list = NULL; > + (*loop_clauses)->device_types = (*orig_clauses)->device_types; > + > + gfc_filter_oacc_combined_clauses (&(*orig_clauses)->dtype_clauses, > + &(*loop_clauses)->dtype_clauses); > +} > + > +/* Combined OpenACC parallel loop and kernels loop. */ > static tree > gfc_trans_oacc_combined_directive (gfc_code *code) > { > stmtblock_t block, inner, *pblock = NULL; > - gfc_omp_clauses construct_clauses, loop_clauses; > + gfc_omp_clauses *loop_clauses; > tree stmt, oacc_clauses = NULL_TREE; > enum tree_code construct_code; > bool scan_nodesc_arrays = false; > @@ -3661,39 +3714,18 @@ gfc_trans_oacc_combined_directive (gfc_code *code) > > gfc_start_block (&block); > > - memset (&loop_clauses, 0, sizeof (loop_clauses)); > - if (code->ext.omp_clauses != NULL) > - { > - memcpy (&construct_clauses, code->ext.omp_clauses, > - sizeof (construct_clauses)); > - loop_clauses.collapse = construct_clauses.collapse; > - loop_clauses.gang = construct_clauses.gang; > - loop_clauses.gang_expr = construct_clauses.gang_expr; > - loop_clauses.gang_static = construct_clauses.gang_static; > - loop_clauses.vector = construct_clauses.vector; > - loop_clauses.vector_expr = construct_clauses.vector_expr; > - loop_clauses.worker = construct_clauses.worker; > - loop_clauses.worker_expr = construct_clauses.worker_expr; > - loop_clauses.seq = construct_clauses.seq; > - loop_clauses.independent = construct_clauses.independent; > - construct_clauses.collapse = 0; > - construct_clauses.gang = false; > - construct_clauses.vector = false; > - construct_clauses.worker = false; > - construct_clauses.seq = false; > - construct_clauses.independent = false; > - oacc_clauses = gfc_trans_omp_clauses (&block, &construct_clauses, > - code->loc); > - } > + gfc_filter_oacc_combined_clauses (&code->ext.omp_clauses, &loop_clauses); > + oacc_clauses = gfc_trans_omp_clauses (&block, code->ext.omp_clauses, > + code->loc); > > array_set = gfc_init_nodesc_arrays (&inner, &oacc_clauses, code, > scan_nodesc_arrays); > With... > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/goacc/combined-directives.f90 ... newly added, and... > --- a/libgomp/testsuite/libgomp.oacc-fortran/combdir-1.f90 > +++ /dev/null ... renamed to... > --- /dev/null > +++ b/libgomp/testsuite/libgomp.oacc-fortran/combined-directive-1.f90 ... this, plus the unaltered libgomp/testsuite/libgomp.oacc-c-c++-common/combdir-1.c, we now got three variants: "combined-directives", "combined-directive", and "combdir". Please settle on one of them. > --- a/gcc/testsuite/gfortran.dg/gomp/intentin1.f90 > +++ b/gcc/testsuite/gfortran.dg/gomp/intentin1.f90 > @@ -11,6 +11,6 @@ subroutine foo (x) > !$omp simd linear (x) ! { dg-error "INTENT.IN. POINTER" } > do i = 1, 10 > end do > -!$omp single ! { dg-error "INTENT.IN. POINTER" } > -!$omp end single copyprivate (x) > +!$omp single > +!$omp end single copyprivate (x) ! { dg-error "INTENT.IN. POINTER" } > end Please revert this unrelated change. Additionally, in r229482 I checked in the following to restore bootstrap builds: [...]/source-gcc/gcc/cp/parser.c:29649:1: error: 'tree_node* require_positive_expr(tree, location_t, const char*)' defined but not used [-Werror=unused-function] require_positive_expr (tree t, location_t loc, const char *str) ^ [...]/source-gcc/gcc/fortran/openmp.c: In function 'match gfc_match_oacc_declare()': [...]/source-gcc/gcc/fortran/openmp.c:1449:30: error: unused variable 'oc' [-Werror=unused-variable] gfc_oacc_declare *new_oc, *oc; ^ [...]/source-gcc/gcc/fortran/openmp.c: In function 'void gfc_resolve_oacc_declare(gfc_namespace*)': [...]/source-gcc/gcc/fortran/openmp.c:4918:7: error: unused variable 'list' [-Werror=unused-variable] int list; ^ commit 1e53d795ea68bcc7e5d5a7fa21e58c506e557cb4 Author: tschwinge Date: Wed Oct 28 10:57:45 2015 +0000 Address -Werror=unused-variable and -Werror=unused-function diagnostics gcc/cp/ * parser.c (require_positive_expr): Remove function. gcc/fortran/ * openmp.c (gfc_match_oacc_declare): Remove oc local variable. (gfc_resolve_oacc_declare): Remove list local variable. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@229482 138bc75d-0d04-0410-961f-82ee72b054a4 --- gcc/cp/ChangeLog.gomp | 4 ++++ gcc/cp/parser.c | 17 ----------------- gcc/fortran/ChangeLog.gomp | 5 +++++ gcc/fortran/openmp.c | 3 +-- 4 files changed, 10 insertions(+), 19 deletions(-) Grüße Thomas diff --git gcc/cp/ChangeLog.gomp gcc/cp/ChangeLog.gomp index c0b96ba..b96bfce 100644 --- gcc/cp/ChangeLog.gomp +++ gcc/cp/ChangeLog.gomp @@ -1,3 +1,7 @@ +2015-10-28 Thomas Schwinge + + * parser.c (require_positive_expr): Remove function. + 2015-10-27 Cesar Philippidis * parser.c (cp_parser_oacc_shape_clause): Backport from trunk. diff --git gcc/cp/parser.c gcc/cp/parser.c index d113cfb..2c6060b 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -29642,23 +29642,6 @@ cp_parser_oacc_data_clause_deviceptr (cp_parser *parser, tree list) return list; } -/* Attempt to statically determine when the number T isn't positive. - Warn if we determined this and return positive one as the new - expression. */ -static tree -require_positive_expr (tree t, location_t loc, const char *str) -{ - tree c = fold_build2_loc (loc, LE_EXPR, boolean_type_node, t, - build_int_cst (TREE_TYPE (t), 0)); - if (c == boolean_true_node) - { - warning_at (loc, 0, - "%<%s%> value must be positive", str); - t = integer_one_node; - } - return t; -} - /* OpenACC: num_gangs ( expression ) num_workers ( expression ) diff --git gcc/fortran/ChangeLog.gomp gcc/fortran/ChangeLog.gomp index d126367..9e1b95b 100644 --- gcc/fortran/ChangeLog.gomp +++ gcc/fortran/ChangeLog.gomp @@ -1,3 +1,8 @@ +2015-10-28 Thomas Schwinge + + * openmp.c (gfc_match_oacc_declare): Remove oc local variable. + (gfc_resolve_oacc_declare): Remove list local variable. + 2015-10-27 Cesar Philippidis * gfortran.h (gfc_omp_namelist): Add locus where member. diff --git gcc/fortran/openmp.c gcc/fortran/openmp.c index 9621eaf..afcce9a 100644 --- gcc/fortran/openmp.c +++ gcc/fortran/openmp.c @@ -1446,7 +1446,7 @@ gfc_match_oacc_declare (void) gfc_omp_clauses *c; gfc_omp_namelist *n; gfc_namespace *ns = gfc_current_ns; - gfc_oacc_declare *new_oc, *oc; + gfc_oacc_declare *new_oc; bool module_var = false; if (gfc_match_omp_clauses (&c, OACC_DECLARE_CLAUSES, 0, false, false, true) @@ -4915,7 +4915,6 @@ resolve_oacc_declare_map (gfc_oacc_declare *declare, int list) void gfc_resolve_oacc_declare (gfc_namespace *ns) { - int list; gfc_omp_namelist *n; locus loc; gfc_oacc_declare *oc;