From patchwork Mon Jul 3 09:22:27 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 783376 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 3x1M9h2PwSz9ryr for ; Mon, 3 Jul 2017 19:22:48 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="WhK2RaHT"; 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:subject:in-reply-to:references:date:message-id:mime-version :content-type:content-transfer-encoding; q=dns; s=default; b=oyB xQ6O6aWCzAHpogD/zXC1pWoDUr+OeR606CvKtWIMOpyaMjFyvYOeGER0uxl2nwy8 QSyjQsU2aYsjlix+A5jrxU3OtRO0/4gIyQQfpzP3ZU5z8qcrDv7tx/sjt3VOgU3s jI3OraSM5Rruj7QS7tnUr7uOtURUsqNHqGl7EXFg= 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:subject:in-reply-to:references:date:message-id:mime-version :content-type:content-transfer-encoding; s=default; bh=3m82PaNka NszAzEOYI0JYCQQYNM=; b=WhK2RaHTpnMikbY7+zL+fZmzPiULPfuUcQomNXRFo KLKy8exmWJSz7dM9uJ0rCdEUeHoiks5bXi2Cd4fbBEUtU8ohB3k6UCDP59AqiCks Y6QRLryJs+G9VEeCUbyamG2ZFeSxLlDdDXghnDhUfOC3K0fktdIi+llg+z8TuuDz mU= Received: (qmail 91100 invoked by alias); 3 Jul 2017 09:22:40 -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 91058 invoked by uid 89); 3 Jul 2017 09:22:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-11.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS, URIBL_RED autolearn=ham version=3.3.2 spammy=gr, gr=c3=bc=c3, accelerator?= 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; Mon, 03 Jul 2017 09:22:36 +0000 Received: from svr-orw-fem-03.mgc.mentorg.com ([147.34.97.39]) by relay1.mentorg.com with esmtp id 1dRxYv-0006db-G5 from Thomas_Schwinge@mentor.com ; Mon, 03 Jul 2017 02:22:33 -0700 Received: from tftp-cs (147.34.91.1) by svr-orw-fem-03.mgc.mentorg.com (147.34.97.39) with Microsoft SMTP Server id 14.3.224.2; Mon, 3 Jul 2017 02:22:33 -0700 Received: by tftp-cs (Postfix, from userid 49978) id A7609C232B; Mon, 3 Jul 2017 02:22:32 -0700 (PDT) From: Thomas Schwinge To: Cesar Philippidis , "gcc-patches@gcc.gnu.org" , Fortran List , Jakub Jelinek Subject: Re: [gomp4] fix an ICE involving assumed-size arrays In-Reply-To: <7a479500-a8ef-7783-a4bb-9910dcb78848@codesourcery.com> References: <7a479500-a8ef-7783-a4bb-9910dcb78848@codesourcery.com> User-Agent: Notmuch/0.9-125-g4686d11 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Mon, 3 Jul 2017 11:22:27 +0200 Message-ID: <87eftx50z0.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Hi! On Tue, 30 Aug 2016 14:55:06 -0700, Cesar Philippidis wrote: > Usually a data clause would would have OMP_CLAUSE_SIZE set, but not all > do. In the latter case, lower_omp_target falls back to using size of the > type of the variable specified in the data clause. However, in the case > of assumed-size arrays, the size of the type may be NULL because its > undefined. My fix for this solution is to set the size to one byte if > the size of the type is NULL. This solution at least allows the runtime > the opportunity to remap any data already present on the accelerator. > However, if the data isn't present on the accelerator, this will likely > result in some sort of segmentation fault on the accelerator. > > The OpenACC spec is not clear how the compiler should handle > assumed-sized arrays when the user does not provide an explicit data > clause with a proper subarray. It was tempting to make such implicit > variables errors, but arguably that would affect usability. Perhaps I > should a warning for implicitly used assumed-sizes arrays? (I don't know a lot about Fortran assumed-size arrays, but I agree that a user might expect code to work, like that in the example you added.) > I've applied this patch to gomp-4_0-branch. It looks like OpenMP has a > similar problem. ... which Jakub for fixed in trunk r243860, by "disallow[ing] explicit or implicit OpenMP mapping of assumed-size arrays". So when merging these two changes, I had to apply the following additional patch, which will need to get resolved some way or another: For reference, here are Cesar's gomp-4_0-branch r239874 changes: > --- a/gcc/omp-low.c > +++ b/gcc/omp-low.c > @@ -16534,6 +16534,12 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) > s = OMP_CLAUSE_SIZE (c); > if (s == NULL_TREE) > s = TYPE_SIZE_UNIT (TREE_TYPE (ovar)); > + /* Fortran assumed-size arrays have zero size because the > + type is incomplete. Set the size to one to allow the > + runtime to remap any existing data that is already > + present on the accelerator. */ > + if (s == NULL_TREE) > + s = integer_one_node; > s = fold_convert (size_type_node, s); > purpose = size_int (map_idx++); > CONSTRUCTOR_APPEND_ELT (vsize, purpose, s); > --- /dev/null > +++ b/libgomp/testsuite/libgomp.oacc-fortran/assumed-size.f90 > @@ -0,0 +1,31 @@ > +! Test if implicitly determined data clauses work with an > +! assumed-sized array variable. Note that the array variable, 'a', > +! has been explicitly copied in and out via acc enter data and acc > +! exit data, respectively. (Should add a "dg-do run" directive here?) > + > +program test > + implicit none > + > + integer, parameter :: n = 100 > + integer a(n), i > + > + call dtest (a, n) > + > + do i = 1, n > + if (a(i) /= i) call abort > + end do > +end program test > + > +subroutine dtest (a, n) > + integer i, n > + integer a(*) > + > + !$acc enter data copyin(a(1:n)) > + > + !$acc parallel loop > + do i = 1, n > + a(i) = i > + end do > + > + !$acc exit data copyout(a(1:n)) > +end subroutine dtest Grüße Thomas --- gcc/fortran/trans-openmp.c +++ gcc/fortran/trans-openmp.c @@ -1048,6 +1048,11 @@ gfc_omp_finish_clause (tree c, gimple_seq *pre_p) tree decl = OMP_CLAUSE_DECL (c); + /* This conflicts with the OpenACC changes done to support assumed-size + arrays that are implicitly mapped after enter data directive (see + libgomp.oacc-fortran/assumed-size.f90) -- doesn't the same apply to + OpenMP, too? */ +#if 0 /* Assumed-size arrays can't be mapped implicitly, they have to be mapped explicitly using array sections. */ if (TREE_CODE (decl) == PARM_DECL @@ -1061,6 +1066,7 @@ gfc_omp_finish_clause (tree c, gimple_seq *pre_p) "implicit mapping of assumed size array %qD", decl); return; } +#endif tree c2 = NULL_TREE, c3 = NULL_TREE, c4 = NULL_TREE; if (POINTER_TYPE_P (TREE_TYPE (decl))) --- gcc/testsuite/gfortran.dg/gomp/pr78866-2.f90 +++ gcc/testsuite/gfortran.dg/gomp/pr78866-2.f90 @@ -3,7 +3,8 @@ subroutine pr78866(x) integer :: x(*) -!$omp target ! { dg-error "implicit mapping of assumed size array" } +! Regarding the XFAIL, see gcc/fortran/trans-openmp.c:gfc_omp_finish_clause. +!$omp target ! { dg-error "implicit mapping of assumed size array" "" { xfail *-*-* } } x(1) = 1 !$omp end target end