From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa4.mentor.iphmx.com (esa4.mentor.iphmx.com [68.232.137.252]) by sourceware.org (Postfix) with ESMTPS id 31B823858C83; Mon, 25 Apr 2022 21:20:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 31B823858C83 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="5.90,289,1643702400"; d="scan'208,223";a="74934548" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa4.mentor.iphmx.com with ESMTP; 25 Apr 2022 13:20:01 -0800 IronPort-SDR: b47yzrFNROIO6j19l/RIVo0rz/IfCSQzNfalaw9Ov1PMrFBo5cdScEZ9ZBl+ZzWgJ440d/bDNw pEybRftgjDQAfdjIB2gHk77rezaxmOX0nz1rNk3dPvDMLdrXtwqfBNEP++etWV9KNJDFS8oqDu bxLuFlzBoY1HJtasg2B31MHFoEhHWcofOyWADmR22Z/YPu2qf3ODuHzKnMJ24BrWLuAMg9PKgc 1ZBsjwKu/9Q9ENEzx99ifA1vD32dLiAL5U2NQVubsMhERlMlCuDFh3+txDBfHuTTtuv3R3dEb9 pDc= From: Thomas Schwinge To: Jakub Jelinek , CC: Tobias Burnus , Subject: Re: [PATCH] fortran: Fix up gfc_trans_oacc_construct [PR104717] In-Reply-To: References: User-Agent: Notmuch/0.29.1+93~g67ed7df (https://notmuchmail.org) Emacs/26.3 (x86_64-pc-linux-gnu) Date: Mon, 25 Apr 2022 23:19:26 +0200 Message-ID: <87tuagsvxd.fsf@dem-tschwing-1.ger.mentorg.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: SVR-IES-MBX-07.mgc.mentorg.com (139.181.222.7) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Apr 2022 21:20:04 -0000 --=-=-= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi! On 2022-04-20T19:06:17+0200, Jakub Jelinek wrote: > So that move_sese_region_to_fn works properly, OpenMP/OpenACC constructs > for which that function is invoked need an extra artificial BIND_EXPR > around their body so that we move all variables of the bodies. > > The C/C++ FEs do that both for OpenMP constructs like OMP_PARALLEL, OMP_T= ASK > or OMP_TARGET and for OpenACC constructs that behave similarly to > OMP_TARGET, but the Fortran FE only does that for OpenMP constructs. > > The following patch does that for OpenACC constructs too. > This fixes ICE on the attached testcase. ACK, thanks. I do see that over the past years, there have been a number of similar fixes been applied for other OMP constructs -- maybe that interface should generally be made more obvious and fool-proof? > Unfortunately, it also regresses > FAIL: gfortran.dg/goacc/privatization-1-compute-loop.f90 -O (test for = excess errors) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=3D= 1 -DACC_MEM_SHARED=3D1 -foffload=3Ddisable -O0 (test for excess errors) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=3D= 1 -DACC_MEM_SHARED=3D1 -foffload=3Ddisable -O1 (test for excess errors) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=3D= 1 -DACC_MEM_SHARED=3D1 -foffload=3Ddisable -O2 (test for excess errors) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=3D= 1 -DACC_MEM_SHARED=3D1 -foffload=3Ddisable -O3 -fomit-frame-pointer -funro= ll-loops -fpeel-loops -ftracer -finline-functions (test for excess errors) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=3D= 1 -DACC_MEM_SHARED=3D1 -foffload=3Ddisable -O3 -g (test for excess errors= ) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=3D= 1 -DACC_MEM_SHARED=3D1 -foffload=3Ddisable -Os (test for excess errors) > Those emits emit tons of various messages and now there are some extra on= es, I've fixed these up. > the previous as well as new ones are mostly on artificial variables creat= ed > by the compiler, so I wonder if we should emit those at all. Yes, I did wonder about that, too. (This affects a lot of test cases.) > Anyway, here it is the patch, appart from those regressions passed > bootstrap/regtested on powerpc64le-linux. > > 2022-04-20 Jakub Jelinek > > PR fortran/104717 > * trans-openmp.cc (gfc_trans_oacc_construct): Wrap construct body > in an extra BIND_EXPR. > > * gfortran.dg/goacc/pr104717.f90: New test. > > --- gcc/fortran/trans-openmp.cc.jj 2022-04-06 09:59:32.729654664 +0200 > +++ gcc/fortran/trans-openmp.cc 2022-04-20 12:48:19.773402677 +0200 > @@ -4444,7 +4444,9 @@ gfc_trans_oacc_construct (gfc_code *code > gfc_start_block (&block); > oacc_clauses =3D gfc_trans_omp_clauses (&block, code->ext.omp_clauses, > code->loc, false, true); > + pushlevel (); > stmt =3D gfc_trans_omp_code (code->block->next, true); > + stmt =3D build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0)); > stmt =3D build2_loc (gfc_get_location (&code->loc), construct_code, > void_type_node, stmt, oacc_clauses); > gfc_add_expr_to_block (&block, stmt); > --- gcc/testsuite/gfortran.dg/goacc/pr104717.f90.jj 2022-04-20 12:53:54= .002748265 +0200 > +++ gcc/testsuite/gfortran.dg/goacc/pr104717.f90 2022-04-20 12:53:12= .811321862 +0200 > @@ -0,0 +1,22 @@ > +! PR fortran/104717 > +! { dg-do compile } > +! { dg-options "-O1 -fopenacc -fstack-arrays" } As the ICE was "during IPA pass: pta", I've added an explicit '-fipa-pta' here. > + > +program main > + implicit none (type, external) > + integer :: j > + integer, allocatable :: A(:) > + > + A =3D [(3*j, j=3D1, 10)] > + call foo (A, size(A)) > + deallocate (A) > +contains > + subroutine foo (array, nn) > + integer :: i, nn > + integer :: array(nn) > + > + !$acc parallel copyout(array) > + array =3D [(-i, i =3D 1, nn)] > + !$acc end parallel > + end subroutine foo > +end Pushed to master branch commit b2202431910e30d8505c94d1cb9341cac7080d10 "fortran: Fix up gfc_trans_oacc_construct [PR104717]", see attached. Gr=C3=BC=C3=9Fe Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename="0001-fortran-Fix-up-gfc_trans_oacc_construct-PR104717.patch" >From b2202431910e30d8505c94d1cb9341cac7080d10 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Wed, 20 Apr 2022 19:06:17 +0200 Subject: [PATCH] fortran: Fix up gfc_trans_oacc_construct [PR104717] So that move_sese_region_to_fn works properly, OpenMP/OpenACC constructs for which that function is invoked need an extra artificial BIND_EXPR around their body so that we move all variables of the bodies. The C/C++ FEs do that both for OpenMP constructs like OMP_PARALLEL, OMP_TASK or OMP_TARGET and for OpenACC constructs that behave similarly to OMP_TARGET, but the Fortran FE only does that for OpenMP constructs. The following patch does that for OpenACC constructs too. PR fortran/104717 gcc/fortran/ * trans-openmp.cc (gfc_trans_oacc_construct): Wrap construct body in an extra BIND_EXPR. gcc/testsuite/ * gfortran.dg/goacc/pr104717.f90: New test. * gfortran.dg/goacc/privatization-1-compute-loop.f90: Adjust. libgomp/ * testsuite/libgomp.oacc-fortran/privatized-ref-2.f90: Adjust. Co-authored-by: Thomas Schwinge --- gcc/fortran/trans-openmp.cc | 2 ++ gcc/testsuite/gfortran.dg/goacc/pr104717.f90 | 22 +++++++++++++++++++ .../goacc/privatization-1-compute-loop.f90 | 7 +++--- .../libgomp.oacc-fortran/privatized-ref-2.f90 | 7 ++++++ 4 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/goacc/pr104717.f90 diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc index 25dde826146..43d59abe9e0 100644 --- a/gcc/fortran/trans-openmp.cc +++ b/gcc/fortran/trans-openmp.cc @@ -4444,7 +4444,9 @@ gfc_trans_oacc_construct (gfc_code *code) gfc_start_block (&block); oacc_clauses = gfc_trans_omp_clauses (&block, code->ext.omp_clauses, code->loc, false, true); + pushlevel (); stmt = gfc_trans_omp_code (code->block->next, true); + stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0)); stmt = build2_loc (gfc_get_location (&code->loc), construct_code, void_type_node, stmt, oacc_clauses); gfc_add_expr_to_block (&block, stmt); diff --git a/gcc/testsuite/gfortran.dg/goacc/pr104717.f90 b/gcc/testsuite/gfortran.dg/goacc/pr104717.f90 new file mode 100644 index 00000000000..4ef16187c84 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/goacc/pr104717.f90 @@ -0,0 +1,22 @@ +! Extracted from 'libgomp.oacc-fortran/privatized-ref-2.f90'. + +! { dg-additional-options "-O1 -fstack-arrays -fipa-pta" } + +program main + implicit none (type, external) + integer :: j + integer, allocatable :: A(:) + + A = [(3*j, j=1, 10)] + call foo (A, size(A)) + deallocate (A) +contains + subroutine foo (array, nn) + integer :: i, nn + integer :: array(nn) + + !$acc parallel copyout(array) + array = [(-i, i = 1, nn)] + !$acc end parallel + end subroutine foo +end diff --git a/gcc/testsuite/gfortran.dg/goacc/privatization-1-compute-loop.f90 b/gcc/testsuite/gfortran.dg/goacc/privatization-1-compute-loop.f90 index 4dfeb7e07a2..13772c185ce 100644 --- a/gcc/testsuite/gfortran.dg/goacc/privatization-1-compute-loop.f90 +++ b/gcc/testsuite/gfortran.dg/goacc/privatization-1-compute-loop.f90 @@ -13,7 +13,7 @@ ! passed to 'incr' may be unset, and in that case, it will be set to [...]", ! so to maintain compatibility with earlier Tcl releases, we manually ! initialize counter variables: -! { dg-line l_dummy[variable c_loop 0] } +! { dg-line l_dummy[variable c_compute 0 c_loop 0] } ! { dg-message "dummy" "" { target iN-VAl-Id } l_dummy } to avoid ! "WARNING: dg-line var l_dummy defined, but not used". @@ -26,7 +26,7 @@ contains integer, parameter :: c = 3 integer, external :: g - !$acc parallel + !$acc parallel ! { dg-line l_compute[incr c_compute] } !$acc loop collapse(2) private(a) private(x, y) ! { dg-line l_loop[incr c_loop] } do i = 1, 20 do j = 1, 25 @@ -46,6 +46,8 @@ contains y = a end do end do + !$acc end parallel + ! { dg-note {variable 'count\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_compute$c_compute } ! { dg-note {variable 'count\.[0-9]+' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_loop$c_loop } ! { dg-note {variable 'i' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_loop$c_loop } ! { dg-note {variable 'j' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_loop$c_loop } @@ -54,6 +56,5 @@ contains ! { dg-note {variable 'y' in 'private' clause is candidate for adjusting OpenACC privatization level} "" { target *-*-* } l_loop$c_loop } ! { dg-note {variable 'C\.[0-9]+' declared in block potentially has improper OpenACC privatization level: 'const_decl'} "TODO" { target *-*-* } l_loop$c_loop } ! { dg-note {variable 'y' ought to be adjusted for OpenACC privatization level: 'vector'} "" { target *-*-* } l_loop$c_loop } - !$acc end parallel end subroutine f end module m diff --git a/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90 b/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90 index 1d91e115d9f..b31f4066152 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90 @@ -53,6 +53,9 @@ contains integer :: array(nn) !$acc parallel copyout(array) ! { dg-line l_compute[incr c_compute] } + ! { dg-note {variable 'atmp\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_compute$c_compute } + ! { dg-note {variable 'shadow_loopvar\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_compute$c_compute } + ! { dg-note {variable 'offset\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_compute$c_compute } ! { dg-note {variable 'S\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_compute$c_compute } array = [(-i, i = 1, nn)] !$acc end parallel @@ -81,6 +84,10 @@ contains integer :: array(:) !$acc parallel copyout(array) ! { dg-line l_compute[incr c_compute] } + ! { dg-note {variable 'atmp\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_compute$c_compute } + ! { dg-note {variable 'parm\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_compute$c_compute } + ! { dg-note {variable 'shadow_loopvar\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_compute$c_compute } + ! { dg-note {variable 'offset\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_compute$c_compute } ! { dg-note {variable 'S\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_compute$c_compute } array = [(-2*i, i = 1, size(array))] !$acc end parallel -- 2.25.1 --=-=-=--