From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26207 invoked by alias); 4 Dec 2018 13:30:42 -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 25951 invoked by uid 89); 4 Dec 2018 13:30:25 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=shape, rank, gfc_symbol X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 04 Dec 2018 13:30:23 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1A40E308424D; Tue, 4 Dec 2018 13:30:13 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-117-214.ams2.redhat.com [10.36.117.214]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1BF0C5DE01; Tue, 4 Dec 2018 13:30:11 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id wB4DU9TQ024630; Tue, 4 Dec 2018 14:30:09 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id wB4DU7Ap024629; Tue, 4 Dec 2018 14:30:07 +0100 Date: Tue, 04 Dec 2018 13:30:00 -0000 From: Jakub Jelinek To: Cesar Philippidis , Thomas Schwinge Cc: Fortran List , "gcc-patches@gcc.gnu.org" Subject: Re: [patch] Add OpenACC Fortran support for deviceptr and variable in common blocks Message-ID: <20181204133007.GO12380@tucnak> Reply-To: Jakub Jelinek References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00176.txt.bz2 On Fri, Jun 29, 2018 at 12:04:58PM -0700, Cesar Philippidis wrote: > 2018-06-29 Cesar Philippidis > James Norris > > gcc/fortran/ > * openmp.c (gfc_match_omp_map_clause): Re-write handling of the > deviceptr clause. Add new common_blocks argument. Propagate it to > gfc_match_omp_variable_list. > (gfc_match_omp_clauses): Update calls to gfc_match_omp_map_clauses. > (resolve_positive_int_expr): Promote the warning to an error. > (check_array_not_assumed): Remove pointer check. > (resolve_oacc_nested_loops): Error on do concurrent loops. > * trans-openmp.c (gfc_omp_finish_clause): Don't create pointer data > mappings for deviceptr clauses. > (gfc_trans_omp_clauses): Likewise. > > gcc/ > * gimplify.c (enum gimplify_omp_var_data): Add GOVD_DEVICETPR. > (oacc_default_clause): Privatize fortran common blocks. > (omp_notice_variable): Add GOVD_DEVICEPTR attribute when appropriate. > Defer the expansion of DECL_VALUE_EXPR for common block decls. > (gimplify_scan_omp_clauses): Add GOVD_DEVICEPTR attribute when > appropriate. > (gimplify_adjust_omp_clauses_1): Set GOMP_MAP_FORCE_DEVICEPTR for > implicit deviceptr mappings. > > gcc/testsuite/ > * c-c++-common/goacc/deviceptr-4.c: Update. > * gfortran.dg/goacc/common-block-1.f90: New test. > * gfortran.dg/goacc/common-block-2.f90: New test. > * gfortran.dg/goacc/loop-2.f95: Update. > * gfortran.dg/goacc/loop-3-2.f95: Update. > * gfortran.dg/goacc/loop-3.f95: Update. > * gfortran.dg/goacc/loop-5.f95: Update. > * gfortran.dg/goacc/pr72715.f90: New test. > * gfortran.dg/goacc/sie.f95: Update. > * gfortran.dg/goacc/tile-1.f90: Update. > * gfortran.dg/gomp/pr77516.f90: Update. > > libgomp/ > * oacc-parallel.c (GOACC_parallel_keyed): Handle Fortran deviceptr > clause. > (GOACC_data_start): Likewise. > * testsuite/libgomp.oacc-fortran/common-block-1.f90: New test. > * testsuite/libgomp.oacc-fortran/common-block-2.f90: New test. > * testsuite/libgomp.oacc-fortran/common-block-3.f90: New test. > * testsuite/libgomp.oacc-fortran/deviceptr-1.f90: New test. > --- a/gcc/fortran/openmp.c > +++ b/gcc/fortran/openmp.c > @@ -914,10 +914,11 @@ omp_inv_mask::omp_inv_mask (const omp_mask &m) : omp_mask (m) > mapping. */ > > static bool > -gfc_match_omp_map_clause (gfc_omp_namelist **list, gfc_omp_map_op map_op) > +gfc_match_omp_map_clause (gfc_omp_namelist **list, gfc_omp_map_op map_op, > + bool common_blocks) > { > gfc_omp_namelist **head = NULL; > - if (gfc_match_omp_variable_list ("", list, false, NULL, &head, true) > + if (gfc_match_omp_variable_list ("", list, common_blocks, NULL, &head, true) > == MATCH_YES) > { > gfc_omp_namelist *n; > @@ -1039,7 +1040,7 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const omp_mask mask, > if ((mask & OMP_CLAUSE_COPY) > && gfc_match ("copy ( ") == MATCH_YES > && gfc_match_omp_map_clause (&c->lists[OMP_LIST_MAP], > - OMP_MAP_TOFROM)) > + OMP_MAP_TOFROM, openacc)) Why? OpenMP doesn't have a copy clause, so I'd expect true here. > continue; > if (mask & OMP_CLAUSE_COPYIN) > { > @@ -1047,7 +1048,7 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const omp_mask mask, > { > if (gfc_match ("copyin ( ") == MATCH_YES > && gfc_match_omp_map_clause (&c->lists[OMP_LIST_MAP], > - OMP_MAP_TO)) > + OMP_MAP_TO, true)) > continue; OpenMP does have a copyin clause, but it is handled below, so this one is ok. > @@ -1156,12 +1157,12 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const omp_mask mask, > && openacc > && gfc_match ("device ( ") == MATCH_YES > && gfc_match_omp_map_clause (&c->lists[OMP_LIST_MAP], > - OMP_MAP_FORCE_TO)) > + OMP_MAP_FORCE_TO, false)) > continue; > if ((mask & OMP_CLAUSE_DEVICEPTR) > && gfc_match ("deviceptr ( ") == MATCH_YES > && gfc_match_omp_map_clause (&c->lists[OMP_LIST_MAP], > - OMP_MAP_FORCE_DEVICEPTR)) > + OMP_MAP_FORCE_DEVICEPTR, false)) > continue; Not sure about these, your call. deviceptr is OpenACC specific clause, device is in both, but means something different in OpenMP. In any case, haven't looked if OpenACC allows common blocks in these clauses. > @@ -3718,8 +3719,8 @@ resolve_positive_int_expr (gfc_expr *expr, const char *clause) > if (expr->expr_type == EXPR_CONSTANT > && expr->ts.type == BT_INTEGER > && mpz_sgn (expr->value.integer) <= 0) > - gfc_warning (0, "INTEGER expression of %s clause at %L must be positive", > - clause, &expr->where); > + gfc_error ("INTEGER expression of %s clause at %L must be positive", > + clause, &expr->where); > } This affects OpenMP too and makes it inconsistent with C/C++. Why? If you need it for OpenACC clauses, then we need two routines. > static void > @@ -3777,10 +3778,6 @@ check_array_not_assumed (gfc_symbol *sym, locus loc, const char *name) > if (sym->as && sym->as->type == AS_ASSUMED_RANK) > gfc_error ("Assumed rank array %qs in %s clause at %L", > sym->name, name, &loc); > - if (sym->as && sym->as->type == AS_DEFERRED && sym->attr.pointer > - && !sym->attr.contiguous) > - gfc_error ("Noncontiguous deferred shape array %qs in %s clause at %L", > - sym->name, name, &loc); > } Perhaps it would help to mention oacc in the name of this routine to make it clearer that it is OpenACC only. The change is ok if that is what OpenACC now allows. > --- a/gcc/gimplify.c > +++ b/gcc/gimplify.c > @@ -105,6 +105,9 @@ enum gimplify_omp_var_data > /* Flag for GOVD_MAP: must be present already. */ > GOVD_MAP_FORCE_PRESENT = 524288, > > + /* Flag for OpenACC deviceptrs. */ > + GOVD_DEVICEPTR = (1<<21), Please use the same style of constants as in the rest (and, you need to adjust anyway for current trunk). Jakub