https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94358
Bug ID: 94358
Summary: [OMP] Privatize internal array variables introduced by
the Fortran FE
Product: gcc
Version: unknown
Status: UNCONFIRMED
Keywords: openacc, openmp
Severity: normal
Priority: P3
Component: fortran
Assignee: unassigned at gcc dot gnu.org
Reporter: tschwinge at gcc dot gnu.org
CC: burnus at gcc dot gnu.org, jakub at gcc dot gnu.org
Target Milestone: ---
This is discussed in context of OpenACC, but I suppose also applies to OpenMP?
Just "a while ago", Cesar submitted for gomp-4_0-branch in
<http://mid.mail-archive.com/561D65A9.3060903@codesourcery.com> "[gomp4]
privatize internal array variables introduced by the fortran FE", and
committed:
gomp-4_0-branch, openacc-gcc-7-branch commit
fb7036c540273b8da507dac254f5fcf2033a8fb1
Author: cesar <cesar@138bc75d-0d04-0410-961f-82ee72b054a4>
AuthorDate: Tue Oct 13 20:17:47 2015 +0000
Commit: cesar <cesar@138bc75d-0d04-0410-961f-82ee72b054a4>
CommitDate: Tue Oct 13 20:17:47 2015 +0000
gcc/fortran/
* trans-array.c (gfc_trans_array_bounds): Add an INIT_VLA
argument
to control whether VLAs should be initialized. Don't mark this
function as static.
(gfc_trans_auto_array_allocation): Update call to
gfc_trans_array_bounds.
(gfc_trans_g77_array): Likewise.
* trans-array.h: Declare gfc_trans_array_bounds.
* trans-openmp.c (gfc_scan_nodesc_arrays): New function.
(gfc_privatize_nodesc_arrays_1): New function.
(gfc_privatize_nodesc_arrays): New function.
(gfc_init_nodesc_arrays): New function.
(gfc_trans_oacc_construct): Initialize any internal variables
for
arrays without array descriptors inside the offloaded parallel
and
kernels region.
(gfc_trans_oacc_combined_directive): Likewise.
gcc/testsuite/
* gfortran.dg/goacc/default_none.f95: New test.
git-svn-id:
svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@228782
138bc75d-0d04-0410-961f-82ee72b054a4
gcc/fortran/ChangeLog.gomp | 18 ++++++++++++++++++
gcc/fortran/trans-array.c | 12 +++++++-----
gcc/fortran/trans-array.h | 2 ++
gcc/fortran/trans-openmp.c | 179
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
gcc/testsuite/ChangeLog.gomp | 4 ++++
gcc/testsuite/gfortran.dg/goacc/default_none.f95 | 59
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 266 insertions(+), 8 deletions(-)
Later, a bug fix got submitted for gomp-4_0-branch in
<http://mid.mail-archive.com/5695AD6B.5060802@codesourcery.com> "[gomp4] arrays
inside modules" (but wrong patch file attached), and committed:
gomp-4_0-branch, openacc-gcc-7-branch commit
b9a99e0c7b39ab4b82f5dd148923f03c0dba7aa4
commit b9a99e0c7b39ab4b82f5dd148923f03c0dba7aa4
Author: cesar <cesar@138bc75d-0d04-0410-961f-82ee72b054a4>
AuthorDate: Wed Jan 13 01:43:16 2016 +0000
Commit: cesar <cesar@138bc75d-0d04-0410-961f-82ee72b054a4>
CommitDate: Wed Jan 13 01:43:16 2016 +0000
gcc/fortran/
* trans-openmp.c (gfc_privatize_nodesc_arrays): Use
gfc_get_symbol_decl instead of accessing backend_decl directly.
gcc/testsuite/
* gfortran.dg/goacc/mod-array.f90: New test.
git-svn-id:
svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@232307
138bc75d-0d04-0410-961f-82ee72b054a4
gcc/fortran/ChangeLog.gomp | 5 +++++
gcc/fortran/trans-openmp.c | 2 +-
gcc/testsuite/ChangeLog.gomp | 4 ++++
gcc/testsuite/gfortran.dg/goacc/mod-array.f90 | 23 +++++++++++++++++++++++
4 files changed, 33 insertions(+), 1 deletion(-)
These two got merged in og8:
openacc-gcc-8-branch commit be4fecc76b4166e4af6bd1d9646393830ba28df4
commit be4fecc76b4166e4af6bd1d9646393830ba28df4
Author: Cesar Philippidis <ce...@codesourcery.com>
AuthorDate: Tue Oct 13 20:17:47 2015 +0000
Commit: Thomas Schwinge <tho...@codesourcery.com>
CommitDate: Wed May 23 08:36:03 2018 +0200
Privatize internal array variables introduced by the fortran FE
gcc/fortran/
* trans-array.c (gfc_trans_array_bounds): Add an INIT_VLA
argument
to control whether VLAs should be initialized. Don't mark this
function as static.
(gfc_trans_auto_array_allocation): Update call to
gfc_trans_array_bounds.
(gfc_trans_g77_array): Likewise.
* trans-array.h: Declare gfc_trans_array_bounds.
* trans-openmp.c (gfc_scan_nodesc_arrays): New function.
(gfc_privatize_nodesc_arrays_1): New function.
(gfc_privatize_nodesc_arrays): New function.
(gfc_init_nodesc_arrays): New function.
(gfc_trans_oacc_construct): Initialize any internal variables
for
arrays without array descriptors inside the offloaded parallel
and
kernels region.
(gfc_trans_oacc_combined_directive): Likewise.
gcc/testsuite/
* gfortran.dg/goacc/mod-array.f90: New test.
(cherry picked from gomp-4_0-branch r228782 and r232307)
gcc/fortran/ChangeLog.openacc | 18 ++++++++++++++++++
gcc/fortran/trans-array.c | 12 +++++++-----
gcc/fortran/trans-array.h | 2 ++
gcc/fortran/trans-openmp.c | 179
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
gcc/testsuite/ChangeLog.openacc | 4 ++++
gcc/testsuite/gfortran.dg/goacc/mod-array.f90 | 23
+++++++++++++++++++++++
6 files changed, 230 insertions(+), 8 deletions(-)
This patch did not make it into og9.
The original test case, 'gcc/testsuite/gfortran.dg/goacc/default_none.f95', had
already gotten committed (unmodified) to trunk in r229832 (commit
ef014f95d542ae7b18cb4150cbb1f1ef68160f7b, 2015-11-06) -- so presumably didn't
exhibit the original 'default(none)' problem any more.
That's presumably due to <http://mid.mail-archive.com/564378BB.8050400@acm.org>
"open acc default data attribute", where per the "fortran FE" comment, the new
'DECL_ARTIFICIAL' handling might indeed be the reason why we're no longer
seeing the original 'default(none)' correctness issue.
But, even under the assumtion that this now no longer is a 'default(none)'
correctness issue, that still leaves the patch's aspect of performance
optimization. Maybe that one is no longer as important as it once was (because
of other performance optimizations being applied, especially on og8 etc., not
yet on master branch), but it conceptually still seems "the right thing to do"
to me?
Given og8 sources, let's consider the '!$acc parallel loop' in
'gcc/testsuite/gfortran.dg/goacc/default_none.f95:foobar'.
Comparing the "baseline" (both og8 commit
7c2f3f5c50e029bec704605b065d63273c2425dd (as a prerequisite, see comment to
follow), and og8 commit be4fecc76b4166e4af6bd1d9646393830ba28df4 reverted) to
"Privatize internal array variables introduced by the fortran FE" (only og8
commit 7c2f3f5c50e029bec704605b065d63273c2425dd reverted):
In the 'original' dump, we see a number of 'private' clauses being added:
'private(stride.4) private(ubound.1) private(size.6) private(ubound.3)
private(lbound.0) private(offset.5) private(lbound.2)', and 'offset.5' as well
as 'D.3818' being re-computed inside the offloaded region, via from 'lbound',
'stride', etc., based on data that is available anyway: 'n'.
In the 'gimple' dump, we then see "baseline": 'firstprivate(D.3818)
firstprivate(offset.5)' (data transfers) vs. "Privatize internal array
variables introduced by the fortran FE": 'private(offset.5)' (no data
transfer), and 'D.3818' has been made local to the offloaded region (no data
transfer).
I have not looked at further optimizations happening, but the theory -- I
suppose -- is that the "Privatize internal array variables introduced by the
fortran FE" variant can then be optimized better, compared to the "baseline"
variant, where we've got "un-optimizable" (or, at least hard-to-optimize) data
transfers due to the two 'firstprivate's.
Even though there may be other ways to achieve this optimization (here: have
the compiler move the 'offset.5' etc. calculations *into* the offloaded region,
and then apply something as discussed in PR90573 "Avoid unnecessary data
transfer into OMP construct" to re-write 'firstprivate' into 'private'), it
seems beneficial to me, to have the front ends produce proper (conceptually
simple, explicit) code as much as possible.
