Hi! On 2019-10-15T23:32:32+0200, Tobias Burnus <tob...@codesourcery.com> wrote: > This OpenACC-only patch extends the support for /common/ blocks.
I'll be quick to note that I don't have any first-hand experience with Fortran common blocks. :-P > [In OpenMP (4.0 to 5.0, unchanged) and gfortran, common blocks are supported > in copyin/copyprivate, in firstprivate/lastprivate/private/shared, in > threadprivate and in declare target.] > > For OpenACC, gfortran already supports common blocks for > device_resident/usedevice/cache/flush/link. > > This patch adds them (for OpenACC only) to copy/copyin/copyout, create/delete, > host, pcopy/pcopy_in/pcopy_out, present_or_copy, present_or_copy_in, > present_or_copy_out, present_or_create and self. > [Of those, only "copy()" is also an OpenMP clause name.] I'm confused: in <http://mid.mail-archive.com/20181204133007.GO12380@tucnak> Jakub stated that "OpenMP doesn't have a copy clause, so I'd expect true here": | @@ -1051,7 +1052,7 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const omp_mask mask, | if ((mask & OMP_CLAUSE_COPY) | && gfc_match ("copy ( ") == MATCH_YES | && gfc_match_omp_map_clause (&c->lists[OMP_LIST_MAP], | - OMP_MAP_TOFROM)) | + OMP_MAP_TOFROM, openacc)) | continue; > [Cf. OpenACC 2.7 in 1.9 (for the p* variants) and 2.13; the latter is new > since OpenACC 2.0.] > > > > I think the Fortran part is obvious, once one agrees on the list of clauses; > and OK from a Fortran's maintainer view. I'll defer to your judgement there, but just one comment: I noticed that OpenACC 2.7 in 2.7. "Data Clauses" states that "For all clauses except 'deviceptr' and 'present', the list argument may include a Fortran _common block_ name enclosed within slashes, if that _common block_ name also appears in a 'declare' directive 'link' clause". Are we already properly handling the aspect that requires that the "that _common block_ name also appears in a 'declare' directive 'link' clause"? The libgomp execution test cases you're adding all state that "This test does not exercise ACC DECLARE", yet they supposedly already do work fine. Or am I understading the OpenACC specification wrongly here? I'm certainly aware of (big) deficiencies in the OpenACC 'declare' handling, so I guess my question here may be whether these test cases are valid after all? > gcc/gimplify.c: oacc_default_clause contains some changes; there are > additionally two lines which only differ for ORT_ACC – Hence, it is an > OpenACC-only change! > The ME change is about privatizing common blocks (I haven't studied this part > closer.) So, please do study that closer. ;-P In <http://mid.mail-archive.com/87efx6haep.fsf@euler.schwinge.homeip.net> I raised some questions, got a bit of an answer, and in <http://mid.mail-archive.com/87bms85kra.fsf@hertz.schwinge.homeip.net> asked further, didn't get an answer. All the rationale from Cesar's original submission email should be transferred into 'gcc/gimplify.c' as much as feasible, to make that "voodoo code" better understandable. > @Jakub, all: comments and approvals are welcome. Indeed. :-) > gcc/ > * gimplify.c (oacc_default_clause): Privatize fortran common blocks. > (omp_notice_variable): Defer the expansion of DECL_VALUE_EXPR for > common block decls. > --- a/gcc/gimplify.c > +++ b/gcc/gimplify.c > @@ -7218,15 +7218,20 @@ oacc_default_clause (struct gimplify_omp_ctx *ctx, > tree decl, unsigned flags) > { > const char *rkind; > bool on_device = false; > + bool is_private = false; > bool declared = is_oacc_declared (decl); > tree type = TREE_TYPE (decl); > > if (lang_hooks.decls.omp_privatize_by_reference (decl)) > type = TREE_TYPE (type); > > + if (RECORD_OR_UNION_TYPE_P (type)) > + is_private = lang_hooks.decls.omp_disregard_value_expr (decl, false); > + > if ((ctx->region_type & (ORT_ACC_PARALLEL | ORT_ACC_KERNELS)) != 0 > && is_global_var (decl) > - && device_resident_p (decl)) > + && device_resident_p (decl) > + && !is_private) > { > on_device = true; > flags |= GOVD_MAP_TO_ONLY; > @@ -7237,7 +7242,9 @@ oacc_default_clause (struct gimplify_omp_ctx *ctx, tree > decl, unsigned flags) > case ORT_ACC_KERNELS: > rkind = "kernels"; > > - if (AGGREGATE_TYPE_P (type)) > + if (is_private) > + flags |= GOVD_MAP; > + else if (AGGREGATE_TYPE_P (type)) > { > /* Aggregates default to 'present_or_copy', or 'present'. */ > if (ctx->default_kind != OMP_CLAUSE_DEFAULT_PRESENT) > @@ -7254,7 +7261,9 @@ oacc_default_clause (struct gimplify_omp_ctx *ctx, tree > decl, unsigned flags) > case ORT_ACC_PARALLEL: > rkind = "parallel"; > > - if (on_device || declared) > + if (is_private) > + flags |= GOVD_FIRSTPRIVATE; > + else if (on_device || declared) > flags |= GOVD_MAP; > else if (AGGREGATE_TYPE_P (type)) > { > @@ -7320,7 +7329,8 @@ omp_notice_variable (struct gimplify_omp_ctx *ctx, tree > decl, bool in_code) > { > tree value = get_base_address (DECL_VALUE_EXPR (decl)); > > - if (value && DECL_P (value) && DECL_THREAD_LOCAL_P (value)) > + if (!(ctx->region_type & ORT_ACC) > + && value && DECL_P (value) && DECL_THREAD_LOCAL_P (value)) > return omp_notice_threadprivate_variable (ctx, decl, value); > } > > @@ -7352,7 +7362,8 @@ omp_notice_variable (struct gimplify_omp_ctx *ctx, tree > decl, bool in_code) > n = splay_tree_lookup (ctx->variables, (splay_tree_key)decl); > if ((ctx->region_type & ORT_TARGET) != 0) > { > - ret = lang_hooks.decls.omp_disregard_value_expr (decl, true); > + shared = !(ctx->region_type & ORT_ACC); > + ret = lang_hooks.decls.omp_disregard_value_expr (decl, shared); > if (n == NULL) > { > unsigned nflags = flags; > @@ -7520,6 +7531,8 @@ omp_notice_variable (struct gimplify_omp_ctx *ctx, tree > decl, bool in_code) > } > > shared = ((flags | n->value) & GOVD_SHARED) != 0; > + if (ctx->region_type & ORT_ACC) > + shared = false; > ret = lang_hooks.decls.omp_disregard_value_expr (decl, shared); > > /* If nothing changed, there's nothing left to do. */ > PS: This patch is the rediffed OG9 (alias OG8) patch > 0793cef408c9937f4c4e2423dd1f7d6a97b9bed3 by Cesar Philippidis from 2016. > (Which was on gomp-4_0-branch as r240165). Due to the wonders of GIT – when > not requiring linear history and due to rebasing with GCC9, it is also part > of the OG9 commit ac6c90812344f4f4cfe4d2f5901c1a9d038a4000 – which in > addition also does some other things like handling OpenACC device pointers. There's no Git magic involved there: somebody just (manually) merged several these patches together into one, for no good reason. ;-\ Grüße Thomas > diff --git a/gcc/testsuite/gfortran.dg/goacc/common-block-1.f90 > b/gcc/testsuite/gfortran.dg/goacc/common-block-1.f90 > new file mode 100644 > index 00000000000..1cbbb49d638 > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/goacc/common-block-1.f90 > @@ -0,0 +1,69 @@ > +! Test data clauses involving common blocks and common block data. > +! Specifically, validates early matching errors. > + > +subroutine subtest > + implicit none > + integer, parameter :: n = 10 > + integer a(n), b(n), c, d(n), e > + real*4 x(n), y(n), z, w(n), v > + common /blockA/ a, c, x > + common /blockB/ b, y, z > + !$acc declare link(/blockA/, /blockB/, e, v) > +end subroutine subtest > + > +program test > + implicit none > + integer, parameter :: n = 10 > + integer a(n), b(n), c, d(n), e > + real*4 x(n), y(n), z, w(n), v > + common /blockA/ a, c, x > + common /blockB/ b, y, z > + !$acc declare link(/blockA/, /blockB/, e, v) > + > + !$acc data copy(/blockA/, /blockB/, e, v) > + !$acc end data > + > + !$acc data copyin(/blockA/, /blockB/, e, v) > + !$acc end data > + > + !$acc data copyout(/blockA/, /blockB/, e, v) > + !$acc end data > + > + !$acc data create(/blockA/, /blockB/, e, v) > + !$acc end data > + > + !$acc data copyout(/blockA/, /blockB/, e, v) > + !$acc end data > + > + !$acc data pcopy(/blockA/, /blockB/, e, v) > + !$acc end data > + > + !$acc data pcopyin(/blockA/, /blockB/, e, v) > + !$acc end data > + > + !$acc data pcopyout(/blockA/, /blockB/, e, v) > + !$acc end data > + > + !$acc data pcreate(/blockA/, /blockB/, e, v) > + !$acc end data > + > + !$acc data pcopyout(/blockA/, /blockB/, e, v) > + !$acc end data > + > + !$acc data present(/blockA/, /blockB/, e, v) ! { dg-error "Syntax error in > OpenMP variable list" } > + !$acc end data ! { dg-error "Unexpected ..ACC END DATA statement" } > + > + !$acc parallel private(/blockA/, /blockB/, e, v) > + !$acc end parallel > + > + !$acc parallel firstprivate(/blockA/, /blockB/, e, v) > + !$acc end parallel > + > + !$acc exit data delete(/blockA/, /blockB/, e, v) > + > + !$acc data deviceptr(/blockA/, /blockB/, e, v) ! { dg-error "Syntax error > in OpenMP variable list" } > + !$acc end data ! { dg-error "Unexpected ..ACC END DATA statement" } > + > + !$acc data deviceptr(/blockA/, /blockB/, e, v) ! { dg-error "Syntax error > in OpenMP variable list" } > + !$acc end data ! { dg-error "Unexpected ..ACC END DATA statement" } > +end program test > diff --git a/gcc/testsuite/gfortran.dg/goacc/common-block-2.f90 > b/gcc/testsuite/gfortran.dg/goacc/common-block-2.f90 > new file mode 100644 > index 00000000000..b83638918a3 > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/goacc/common-block-2.f90 > @@ -0,0 +1,49 @@ > +! Test data clauses involving common blocks and common block data. > +! Specifically, resolver errors such as duplicate data clauses. > + > +program test > + implicit none > + integer, parameter :: n = 10 > + integer a(n), b(n), c, d(n), e > + real*4 x(n), y(n), z, w(n), v > + common /blockA/ a, c, x > + common /blockB/ b, y, z > + > + !$acc data copy(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol .a. > present on multiple clauses" } > + !$acc end data > + > + !$acc data copyin(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol .a. > present on multiple clauses" } > + !$acc end data > + > + !$acc data copyout(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol .a. > present on multiple clauses" } > + !$acc end data > + > + !$acc data create(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol .a. > present on multiple clauses" } > + !$acc end data > + > + !$acc data copyout(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol .a. > present on multiple clauses" } > + !$acc end data > + > + !$acc data pcopy(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol .a. > present on multiple clauses" } > + !$acc end data > + > + !$acc data pcopyin(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol .a. > present on multiple clauses" } > + !$acc end data > + > + !$acc data pcopyout(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol .a. > present on multiple clauses" } > + !$acc end data > + > + !$acc data pcreate(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol .a. > present on multiple clauses" } > + !$acc end data > + > + !$acc data pcopyout(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol .a. > present on multiple clauses" } > + !$acc end data > + > + !$acc parallel private(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol > .a. present on multiple clauses" } > + !$acc end parallel > + > + !$acc parallel firstprivate(/blockA/, /blockB/, e, v, a) ! { dg-error > "Symbol .a. present on multiple clauses" } > + !$acc end parallel > + > + !$acc exit data delete(/blockA/, /blockB/, e, v, a) ! { dg-error "Symbol > .a. present on multiple clauses" } > +end program test > diff --git a/libgomp/testsuite/libgomp.oacc-fortran/common-block-1.f90 > b/libgomp/testsuite/libgomp.oacc-fortran/common-block-1.f90 > new file mode 100644 > index 00000000000..a17a33536f3 > --- /dev/null > +++ b/libgomp/testsuite/libgomp.oacc-fortran/common-block-1.f90 > @@ -0,0 +1,105 @@ > +! Test data located inside common blocks. This test does not exercise > +! ACC DECLARE. > + > +module const > + integer, parameter :: n = 100 > +end module const > + > +subroutine check > + use const > + > + implicit none > + integer i, x(n), y > + common /BLOCK/ x, y > + > + do i = 1, n > + if (x(i) .ne. y) call abort > + end do > +end subroutine check > + > +module m > + use const > + integer a(n), b > + common /BLOCK/ a, b > + > +contains > + subroutine mod_implicit_incr > + implicit none > + integer i > + > + !$acc parallel loop > + do i = 1, n > + a(i) = b > + end do > + !$acc end parallel loop > + > + call check > + end subroutine mod_implicit_incr > + > + subroutine mod_explicit_incr > + implicit none > + integer i > + > + !$acc parallel loop copy(a(1:n)) copyin(b) > + do i = 1, n > + a(i) = b > + end do > + !$acc end parallel loop > + > + call check > + end subroutine mod_explicit_incr > +end module m > + > +subroutine sub_implicit_incr > + use const > + > + implicit none > + integer i, x(n), y > + common /BLOCK/ x, y > + > + !$acc parallel loop > + do i = 1, n > + x(i) = y > + end do > + !$acc end parallel loop > + > + call check > +end subroutine sub_implicit_incr > + > +subroutine sub_explicit_incr > + use const > + > + implicit none > + integer i, x(n), y > + common /BLOCK/ x, y > + > + !$acc parallel loop copy(x(1:n)) copyin(y) > + do i = 1, n > + x(i) = y > + end do > + !$acc end parallel loop > + > + call check > +end subroutine sub_explicit_incr > + > +program main > + use m > + > + implicit none > + > + a(:) = -1 > + b = 5 > + call mod_implicit_incr > + > + a(:) = -2 > + b = 6 > + call mod_explicit_incr > + > + a(:) = -3 > + b = 7 > + call sub_implicit_incr > + > + a(:) = -4 > + b = 8 > + call sub_explicit_incr > +end program main > diff --git a/libgomp/testsuite/libgomp.oacc-fortran/common-block-2.f90 > b/libgomp/testsuite/libgomp.oacc-fortran/common-block-2.f90 > new file mode 100644 > index 00000000000..e27a225a024 > --- /dev/null > +++ b/libgomp/testsuite/libgomp.oacc-fortran/common-block-2.f90 > @@ -0,0 +1,150 @@ > +! Test data located inside common blocks. This test does not exercise > +! ACC DECLARE. All data clauses are explicit. > + > +module consts > + integer, parameter :: n = 100 > +end module consts > + > +subroutine validate > + use consts > + > + implicit none > + integer i, j > + real*4 x(n), y(n), z > + common /BLOCK/ x, y, z, j > + > + do i = 1, n > + if (abs(x(i) - i - z) .ge. 0.0001) call abort > + end do > +end subroutine validate > + > +subroutine incr > + use consts > + > + implicit none > + integer i, j > + real*4 x(n), y(n), z > + common /BLOCK/ x, y, z, j > + > + !$acc parallel loop pcopy(/BLOCK/) > + do i = 1, n > + x(i) = x(i) + z > + end do > + !$acc end parallel loop > +end subroutine incr > + > +program main > + use consts > + > + implicit none > + integer i, j > + real*4 a(n), b(n), c > + common /BLOCK/ a, b, c, j > + > + ! Test copyout, pcopy, device > + > + !$acc data copyout(a, c) > + > + c = 1.0 > + > + !$acc update device(c) > + > + !$acc parallel loop pcopy(a) > + do i = 1, n > + a(i) = i > + end do > + !$acc end parallel loop > + > + call incr > + call incr > + call incr > + !$acc end data > + > + c = 3.0 > + call validate > + > + ! Test pcopy without copyout > + > + c = 2.0 > + call incr > + c = 5.0 > + call validate > + > + ! Test create, delete, host, copyout, copyin > + > + !$acc enter data create(b) > + > + !$acc parallel loop pcopy(b) > + do i = 1, n > + b(i) = i > + end do > + !$acc end parallel loop > + > + !$acc update host (b) > + > + !$acc parallel loop pcopy(b) copyout(a) copyin(c) > + do i = 1, n > + a(i) = b(i) + c > + end do > + !$acc end parallel loop > + > + !$acc exit data delete(b) > + > + call validate > + > + a(:) = b(:) > + c = 0.0 > + call validate > + > + ! Test copy > + > + c = 1.0 > + !$acc parallel loop copy(/BLOCK/) > + do i = 1, n > + a(i) = b(i) + c > + end do > + !$acc end parallel loop > + > + call validate > + > + ! Test pcopyin, pcopyout FIXME > + > + c = 2.0 > + !$acc data copyin(b, c) copyout(a) > + > + !$acc parallel loop pcopyin(b, c) pcopyout(a) > + do i = 1, n > + a(i) = b(i) + c > + end do > + !$acc end parallel loop > + > + !$acc end data > + > + call validate > + > + ! Test reduction, private > + > + j = 0 > + > + !$acc parallel private(i) copy(j) > + !$acc loop reduction(+:j) > + do i = 1, n > + j = j + 1 > + end do > + !$acc end parallel > + > + if (j .ne. n) call abort > + > + ! Test firstprivate, copy > + > + a(:) = 0 > + c = j > + > + !$acc parallel loop firstprivate(c) copyout(a) > + do i = 1, n > + a(i) = i + c > + end do > + !$acc end parallel loop > + > + call validate > +end program main > diff --git a/libgomp/testsuite/libgomp.oacc-fortran/common-block-3.f90 > b/libgomp/testsuite/libgomp.oacc-fortran/common-block-3.f90 > new file mode 100644 > index 00000000000..90448d2da72 > --- /dev/null > +++ b/libgomp/testsuite/libgomp.oacc-fortran/common-block-3.f90 > @@ -0,0 +1,137 @@ > +! Test data located inside common blocks. This test does not exercise > +! ACC DECLARE. Most of the data clauses are implicit. > + > +module consts > + integer, parameter :: n = 100 > +end module consts > + > +subroutine validate > + use consts > + > + implicit none > + integer i, j > + real*4 x(n), y(n), z > + common /BLOCK/ x, y, z, j > + > + do i = 1, n > + if (abs(x(i) - i - z) .ge. 0.0001) call abort > + end do > +end subroutine validate > + > +subroutine incr_parallel > + use consts > + > + implicit none > + integer i, j > + real*4 x(n), y(n), z > + common /BLOCK/ x, y, z, j > + > + !$acc parallel loop > + do i = 1, n > + x(i) = x(i) + z > + end do > + !$acc end parallel loop > +end subroutine incr_parallel > + > +subroutine incr_kernels > + use consts > + > + implicit none > + integer i, j > + real*4 x(n), y(n), z > + common /BLOCK/ x, y, z, j > + > + !$acc kernels > + do i = 1, n > + x(i) = x(i) + z > + end do > + !$acc end kernels > +end subroutine incr_kernels > + > +program main > + use consts > + > + implicit none > + integer i, j > + real*4 a(n), b(n), c > + common /BLOCK/ a, b, c, j > + > + !$acc data copyout(a, c) > + > + c = 1.0 > + > + !$acc update device(c) > + > + !$acc parallel loop > + do i = 1, n > + a(i) = i > + end do > + !$acc end parallel loop > + > + call incr_parallel > + call incr_parallel > + call incr_parallel > + !$acc end data > + > + c = 3.0 > + call validate > + > + ! Test pcopy without copyout > + > + c = 2.0 > + call incr_kernels > + c = 5.0 > + call validate > + > + !$acc kernels > + do i = 1, n > + b(i) = i > + end do > + !$acc end kernels > + > + !$acc parallel loop > + do i = 1, n > + a(i) = b(i) + c > + end do > + !$acc end parallel loop > + > + call validate > + > + a(:) = b(:) > + c = 0.0 > + call validate > + > + ! Test copy > + > + c = 1.0 > + !$acc parallel loop > + do i = 1, n > + a(i) = b(i) + c > + end do > + !$acc end parallel loop > + > + call validate > + > + c = 2.0 > + !$acc data copyin(b, c) copyout(a) > + > + !$acc kernels > + do i = 1, n > + a(i) = b(i) + c > + end do > + !$acc end kernels > + > + !$acc end data > + > + call validate > + > + j = 0 > + > + !$acc parallel loop reduction(+:j) > + do i = 1, n > + j = j + 1 > + end do > + !$acc end parallel loop > + > + if (j .ne. n) call abort > +end program main
signature.asc
Description: PGP signature
