From patchwork Wed Dec 9 16:34:12 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Norris X-Patchwork-Id: 554711 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 C4C7714016A for ; Thu, 10 Dec 2015 03:34:41 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=Fazae8aI; 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=XrNtArz0zLqSO2/Ln gkOr9Gr4cENzF0nJyubTJ11K7NRAU+Dce4aaYWYs1Ow48nQy93Sdt1tjl0Dump16 q0/5TNDoctxwi8slMFlhbHEOpo420jSiKIic3LkZEkXplnV0PEyIiHNEerqc9CDn ewjqV+9vvLseQlX9vtPXbOn8Ck= 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=j6SewDr2BcwYXhuXdtE1GR7 9kDo=; b=Fazae8aIxBQs/q8G3e1ouQUjB5zLIQQ3z7zBJqeYOTYwUAOzX648q6w +G8rYyjLL+0mP+YtEOz9jXREB67UUb10NbflJz29f5oo8bfHlO3r5o4VbT6xyHWi fDFBiHstHYKv8Xhr82hcI+KcoPV68gc7LEftytyATi0ygEbgBkY0= Received: (qmail 77289 invoked by alias); 9 Dec 2015 16:34:28 -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 77270 invoked by uid 89); 9 Dec 2015 16:34:27 -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-Spam-User: qpsmtpd, 2 recipients 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, 09 Dec 2015 16:34:17 +0000 Received: from svr-orw-fem-06.mgc.mentorg.com ([147.34.97.120]) by relay1.mentorg.com with esmtp id 1a6hh0-0003LX-0A from James_Norris@mentor.com ; Wed, 09 Dec 2015 08:34:14 -0800 Received: from [172.30.80.173] (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, 9 Dec 2015 08:34:13 -0800 Message-ID: <56685804.4060901@codesourcery.com> Date: Wed, 9 Dec 2015 10:34:12 -0600 From: James Norris User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Cesar Philippidis , GCC Patches CC: , Thomas Schwinge Subject: Re: [gomp4] Fix Fortran deviceptr References: <56644BBC.1050602@codesourcery.com> <5665AC01.2020008@codesourcery.com> <566703BC.7030202@codesourcery.com> <56670F21.8090802@codesourcery.com> In-Reply-To: <56670F21.8090802@codesourcery.com> X-TagToolbar-Keys: D20151209103412025 Cesar, On 12/08/2015 11:10 AM, Cesar Philippidis wrote: > On 12/08/2015 08:22 AM, James Norris wrote: > >>> 2. It appears that deviceptr code in GOACC_parallel_keyed is mostly >>> identical to GOACC_data_start. Can you put that duplicate code into >>> a function? That would be easier to maintain in the long run. >>> >> >> Fixed. > > Where? I don't see a patch. Oppssss... Now attached. > > Since you're working on fortran, can you take a look at how > gfc_match_omp_clauses is handling OMP_CLAUSE_DEVICEPTR. It seems overly > confusing to me. Could it be done in a similar way as OMP_CLAUSE_COPYIN, > i.e., using gfc_match_omp_map_clause? > Confusion removed and replaced with code similar to how the other clauses are handled. Patch to be committed after testing completes. Thanks! Jim diff --git a/gcc/fortran/ChangeLog.gomp b/gcc/fortran/ChangeLog.gomp index 00e5746..d05326d 100644 --- a/gcc/fortran/ChangeLog.gomp +++ b/gcc/fortran/ChangeLog.gomp @@ -1,3 +1,8 @@ +2015-12-09 James Norris + + * openmp.c (gfc_match_omp_clauses): Re-write handling of the + deviceptr clause. + 2015-12-08 Thomas Schwinge * gfortran.h (symbol_attribute): Add oacc_function_nohost member. diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c index b59528be..78d2e1e 100644 --- a/gcc/fortran/openmp.c +++ b/gcc/fortran/openmp.c @@ -818,19 +818,10 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, uint64_t mask, OMP_MAP_ALLOC)) continue; if ((mask & OMP_CLAUSE_DEVICEPTR) - && gfc_match ("deviceptr ( ") == MATCH_YES) - { - gfc_omp_namelist **list = &c->lists[OMP_LIST_MAP]; - gfc_omp_namelist **head = NULL; - if (gfc_match_omp_variable_list ("", list, true, NULL, &head, false) - == MATCH_YES) - { - gfc_omp_namelist *n; - for (n = *head; n; n = n->next) - n->u.map_op = OMP_MAP_FORCE_DEVICEPTR; - continue; - } - } + && gfc_match ("deviceptr ( ") == MATCH_YES + && gfc_match_omp_map_clause (&c->lists[OMP_LIST_MAP], + OMP_MAP_FORCE_DEVICEPTR)) + continue; if ((mask & OMP_CLAUSE_BIND) && c->routine_bind == NULL && gfc_match ("bind ( %s )", &c->routine_bind) == MATCH_YES) { diff --git a/libgomp/ChangeLog.gomp b/libgomp/ChangeLog.gomp index d88cb94..ec133a7 100644 --- a/libgomp/ChangeLog.gomp +++ b/libgomp/ChangeLog.gomp @@ -1,3 +1,11 @@ +2015-12-09 James Norris + + * oacc-parallel.c (handle_ftn_pointers): New function. + (GOACC_parallel_keyed, GOACC_data_start): Factor out Fortran pointer + handling. + * testsuite/libgomp.oacc-fortran/declare-1.f90: Add comment. + * testsuite/libgomp.oacc-fortran/deviceptr-1.f90: Fix comment. + 2015-12-08 James Norris * testsuite/libgomp.oacc-fortran/kernels-map-1.f90: Add new test. diff --git a/libgomp/oacc-parallel.c b/libgomp/oacc-parallel.c index a606152..f68db78 100644 --- a/libgomp/oacc-parallel.c +++ b/libgomp/oacc-parallel.c @@ -57,6 +57,51 @@ find_pointer (int pos, size_t mapnum, unsigned short *kinds) return 0; } +/* Handle the mapping pair that are presented when a + deviceptr clause is used with Fortran. */ + +static void +handle_ftn_pointers (size_t mapnum, void **hostaddrs, size_t *sizes, + unsigned short *kinds) +{ + int i; + + for (i = 0; i < mapnum; i++) + { + unsigned short kind1 = kinds[i] & 0xff; + + /* Handle Fortran deviceptr clause. */ + if (kind1 == GOMP_MAP_FORCE_DEVICEPTR) + { + unsigned short kind2; + + if (i < (signed)mapnum - 1) + kind2 = kinds[i + 1] & 0xff; + else + kind2 = 0xffff; + + if (sizes[i] == sizeof (void *)) + continue; + + /* At this point, we're dealing with a Fortran deviceptr. + If the next element is not what we're expecting, then + this is an instance of where the deviceptr variable was + not used within the region and the pointer was removed + by the gimplifier. */ + if (kind2 == GOMP_MAP_POINTER + && sizes[i + 1] == 0 + && hostaddrs[i] == *(void **)hostaddrs[i + 1]) + { + kinds[i+1] = kinds[i]; + sizes[i+1] = sizeof (void *); + } + + /* Invalidate the entry. */ + hostaddrs[i] = NULL; + } + } +} + static void goacc_wait (int async, int num_waits, va_list *ap); @@ -99,40 +144,7 @@ GOACC_parallel_keyed (int device, void (*fn) (void *), thr = goacc_thread (); acc_dev = thr->dev; - for (i = 0; i < mapnum; i++) - { - unsigned short kind1 = kinds[i] & 0xff; - - /* Handle Fortran deviceptr clause. */ - if (kind1 == GOMP_MAP_FORCE_DEVICEPTR) - { - unsigned short kind2; - - if (i < (signed)mapnum - 1) - kind2 = kinds[i + 1] & 0xff; - else - kind2 = 0xffff; - - if (sizes[i] == sizeof (void *)) - continue; - - /* At this point, we're dealing with a Fortran deviceptr. - If the next element is not what we're expecting, then - this is an instance of where the deviceptr variable was - not used within the region and the pointer was removed - by the gimplifier. */ - if (kind2 == GOMP_MAP_POINTER - && sizes[i + 1] == 0 - && hostaddrs[i] == *(void **)hostaddrs[i + 1]) - { - kinds[i+1] = kinds[i]; - sizes[i+1] = sizeof (void *); - } - - /* Invalidate the entry. */ - hostaddrs[i] = NULL; - } - } + handle_ftn_pointers (mapnum, hostaddrs, sizes, kinds); /* Host fallback if "if" clause is false or if the current device is set to the host. */ @@ -258,7 +270,6 @@ GOACC_data_start (int device, size_t mapnum, { bool host_fallback = device == GOMP_DEVICE_HOST_FALLBACK; struct target_mem_desc *tgt; - int i; #ifdef HAVE_INTTYPES_H gomp_debug (0, "%s: mapnum=%"PRIu64", hostaddrs=%p, sizes=%p, kinds=%p\n", @@ -273,41 +284,7 @@ GOACC_data_start (int device, size_t mapnum, struct goacc_thread *thr = goacc_thread (); struct gomp_device_descr *acc_dev = thr->dev; - for (i = 0; i < mapnum; i++) - { - unsigned short kind1 = kinds[i] & 0xff; - - /* Handle Fortran deviceptr clause. */ - if (kind1 == GOMP_MAP_FORCE_DEVICEPTR) - { - unsigned short kind2; - - if (i < (signed)mapnum - 1) - kind2 = kinds[i + 1] & 0xff; - else - kind2 = 0xffff; - - /* If the size is right, skip it. */ - if (sizes[i] == sizeof (void *)) - continue; - - /* At this point, we're dealing with a Fortran deviceptr. - If the next element is not what we're expecting, then - this is an instance of where the deviceptr variable was - not used within the region and the pointer was removed - by the gimplifier. */ - if (kind2 == GOMP_MAP_POINTER - && sizes[i + 1] == 0 - && hostaddrs[i] == *(void **)hostaddrs[i + 1]) - { - kinds[i+1] = kinds[i]; - sizes[i+1] = sizeof (void *); - } - - /* Invalidate the entry. */ - hostaddrs[i] = NULL; - } - } + handle_ftn_pointers (mapnum, hostaddrs, sizes, kinds); /* Host fallback or 'do nothing'. */ if ((acc_dev->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM) diff --git a/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90 index e781878..2d4b707 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/declare-1.f90 @@ -1,5 +1,16 @@ ! { dg-do run { target openacc_nvidia_accel_selected } } +! Tests to exercise the declare directive along with +! the clauses: copy +! copyin +! copyout +! create +! present +! present_or_copy +! present_or_copyin +! present_or_copyout +! present_or_create + module vars implicit none integer z diff --git a/libgomp/testsuite/libgomp.oacc-fortran/deviceptr-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/deviceptr-1.f90 index 879cbf1..276a172 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/deviceptr-1.f90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/deviceptr-1.f90 @@ -1,8 +1,8 @@ ! { dg-do run } -!! Test the deviceptr clause with various directives -!! and in combination with other directives where -!! the deviceptr variable is implied. +! Test the deviceptr clause with various directives +! and in combination with other directives where +! the deviceptr variable is implied. subroutine subr1 (a, b) implicit none