From patchwork Fri Jan 10 01:49:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julian Brown X-Patchwork-Id: 1220742 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-517068-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha1 header.s=default header.b=uz3EL1En; dkim-atps=neutral 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 47v5Xh14jvz9sR1 for ; Fri, 10 Jan 2020 12:50:26 +1100 (AEDT) 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:subject:message-id:mime-version:content-type; q=dns; s= default; b=kOgIQa02QgZw4oewfDcUFBnROWmn31uElAD3+liaysiqbuch6ElWG xm+bJ+eJRhbMer3Kot2J1OLtRUJTsqD/cYOFZQXi40DI0LBBAG/uff9esnY5SPln iVb71jLiMd+3BR/FwlS4SqWvu0gW77HoHYc8dgNgY7u51uPSzbJZl4= 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:subject:message-id:mime-version:content-type; s= default; bh=iM3ZVPN7m1LcXRPNsYwXHm2zxAw=; b=uz3EL1EnArF2kbZ6aG75 9TzJ/3zF7Ua7ZfHGnd2J8DnJ2c99IYYmZBqyfo4NBUfRrWdOU41dw5nHB2egqvYo JHMob4752VIXH9hJb4TdLSOrhXzhFkbJdykw3LuRI+iwLeZ/Y4GVsiyLYEQmVgkC wLj5sKnfpyti9rwvpPDn30E= Received: (qmail 66205 invoked by alias); 10 Jan 2020 01:50:03 -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 66124 invoked by uid 89); 10 Jan 2020 01:50:00 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_PASS autolearn=ham version=3.3.1 spammy=copyin, sk:fdumpt, x.a.i, sk:fdump-t X-HELO: esa1.mentor.iphmx.com Received: from esa1.mentor.iphmx.com (HELO esa1.mentor.iphmx.com) (68.232.129.153) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 10 Jan 2020 01:49:58 +0000 IronPort-SDR: aZtML8KdnT/RgFAtd2sCFUYrwU0HonAJd/GQ8Qf4hXUhTEQZt0P9+0rDly7CaDPmIgKhpFSLMm Ff83xz3JHiEmYBT281QIOtokoEXhF5WFH1rGQCLerXgNeUxfY9xFhyMOFMSm1tcx6Bp93/b1+z n9XsFn18uzW0MlccYyac8iSAymSWtOXLLC5EU7jMZeAduG2PfRt7/ucN/MMqFnXG+Cp3epfjCS 74+P6Q7h/8gLRVUmNykGgGjqLceV7TFQbOTY2+M25SxJlkweTNDMI5cohzRE5uuV7LjIRJkZi3 yvs= Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa1.mentor.iphmx.com with ESMTP; 09 Jan 2020 17:49:56 -0800 IronPort-SDR: HqRXtTfJc0i6Kuw12VvZfslzV+MWEE0A+JbEXMFKYCIptJSokdNWRwmom+XQ/GFUiGXACiUBNm XGCI7LhirI01idP5cf786vpmNUdHsojqOrBSj0zA/nlM+ss3jndVVwYRg5klzr9UEH/w6Bqzsm HvfFnzjE+5b/D12lPxYPZREN/aPNu18eZgFmbjsZGsWpOF+GWinpH/RD5FyzkEEeE2So79qaOa MhKnj6VTEwfrqbDLNZ20EKOLEUWBIzr9eZ29ZSOZZZPjvhrk0wRpGoXS/C2tmpJSwiOkkUJYKa Erw= Date: Fri, 10 Jan 2020 01:49:45 +0000 From: Julian Brown To: , Tobias Burnus , fortran , Thomas Schwinge Subject: [PATCH] Fix component mappings with derived types for OpenACC Message-ID: <20200110014945.5643ace5@squid.athome> MIME-Version: 1.0 X-IsSubscribed: yes Hi, This patch fixes a bug with mapping Fortran components (i.e. with the manual deep-copy support) which themselves have derived types. I've also added a couple of new tests to make sure such mappings are lowered correctly, and to check for the case that Tobias found in the message: https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00215.html The previous incorrect mapping was causing the error condition mentioned in that message to fail to trigger, and I think (my!) code in libgomp (goacc_exit_data_internal) to handle GOMP_MAP_STRUCT specially was papering over the bad mapping also. Oops! I haven't attempted to implement the (harder) sub-copy detection, if that is indeed supposed to be forbidden by the spec. This patch should get us to the same behaviour in Fortran as in C & C++ though. Tested with offloading to nvptx, also with the (uncommitted) reference-count self-checking patch enabled. OK? Thanks, Julian ChangeLog gcc/fortran/ * trans-openmp.c (gfc_trans_omp_clauses): Use inner not decl for components with derived types. gcc/testsuite/ * gfortran.dg/goacc/mapping-tests-3.f90: New test. * gfortran.dg/goacc/mapping-tests-4.f90: New test. libgomp/ * oacc-mem.c (goacc_exit_data_internal): Remove special (no-copyback) behaviour for GOMP_MAP_STRUCT. commit 5e9d8846bbaa33a9bdb08adcf1ee9f224a8e8fc0 Author: Julian Brown Date: Wed Jan 8 15:57:46 2020 -0800 Fix component mappings with derived types for OpenACC gcc/fortran/ * trans-openmp.c (gfc_trans_omp_clauses): Use inner not decl for components with derived types. gcc/testsuite/ * gfortran.dg/goacc/mapping-tests-3.f90: New test. * gfortran.dg/goacc/mapping-tests-4.f90: New test. libgomp/ * oacc-mem.c (goacc_exit_data_internal): Remove special (no-copyback) behaviour for GOMP_MAP_STRUCT. diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c index 9835d2aae3c..efc392d7fa6 100644 --- a/gcc/fortran/trans-openmp.c +++ b/gcc/fortran/trans-openmp.c @@ -2783,9 +2783,9 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses, } else { - OMP_CLAUSE_DECL (node) = decl; + OMP_CLAUSE_DECL (node) = inner; OMP_CLAUSE_SIZE (node) - = TYPE_SIZE_UNIT (TREE_TYPE (decl)); + = TYPE_SIZE_UNIT (TREE_TYPE (inner)); } } else if (lastcomp->next diff --git a/gcc/testsuite/gfortran.dg/goacc/mapping-tests-3.f90 b/gcc/testsuite/gfortran.dg/goacc/mapping-tests-3.f90 new file mode 100644 index 00000000000..312f596461e --- /dev/null +++ b/gcc/testsuite/gfortran.dg/goacc/mapping-tests-3.f90 @@ -0,0 +1,15 @@ +! { dg-options "-fopenacc -fdump-tree-omplower" } + +subroutine foo + type one + integer i, j + end type + type two + type(one) A, B + end type + + type(two) x + + !$acc enter data copyin(x%A) +! { dg-final { scan-tree-dump-times "omp target oacc_enter_exit_data map\\(struct:x \\\[len: 1\\\]\\) map\\(to:x.a \\\[len: \[0-9\]+\\\]\\)" 1 "omplower" } } +end diff --git a/gcc/testsuite/gfortran.dg/goacc/mapping-tests-4.f90 b/gcc/testsuite/gfortran.dg/goacc/mapping-tests-4.f90 new file mode 100644 index 00000000000..6257af942df --- /dev/null +++ b/gcc/testsuite/gfortran.dg/goacc/mapping-tests-4.f90 @@ -0,0 +1,17 @@ +subroutine foo + type one + integer i, j + end type + type two + type(one) A, B + end type + + type(two) x + +! This is accepted at present, although it represents a probably-unintentional +! overlapping subcopy. + !$acc enter data copyin(x%A, x%A%i) +! But this raises an error. + !$acc enter data copyin(x%A, x%A%i, x%A%i) +! { dg-error ".x.a.i. appears more than once in map clauses" "" { target "*-*-*" } 15 } +end diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c index 2d4bba78efd..232683a85f0 100644 --- a/libgomp/oacc-mem.c +++ b/libgomp/oacc-mem.c @@ -1136,38 +1136,6 @@ goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum, break; case GOMP_MAP_STRUCT: - { - int elems = sizes[i]; - for (int j = 1; j <= elems; j++) - { - struct splay_tree_key_s k; - k.host_start = (uintptr_t) hostaddrs[i + j]; - k.host_end = k.host_start + sizes[i + j]; - splay_tree_key str; - str = splay_tree_lookup (&acc_dev->mem_map, &k); - if (str) - { - if (finalize) - { - if (str->refcount != REFCOUNT_INFINITY) - str->refcount -= str->virtual_refcount; - str->virtual_refcount = 0; - } - if (str->virtual_refcount > 0) - { - if (str->refcount != REFCOUNT_INFINITY) - str->refcount--; - str->virtual_refcount--; - } - else if (str->refcount > 0 - && str->refcount != REFCOUNT_INFINITY) - str->refcount--; - if (str->refcount == 0) - gomp_remove_var_async (acc_dev, str, aq); - } - } - i += elems; - } break; default: