| Message ID | df64152a77df6ee4df462ee141170e7e5d4b654b.1612271846.git.julian@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | openacc: Mixing array elements and derived types | expand |
LGTM. However, I'd feel better if there were a testcase, which actually checks that it works correctly. (I think that means a libgomp/testsuite/ run test.) Tobias On 02.02.21 14:28, Julian Brown wrote: > OpenACC 3.0 ("2.14.4. Update Directive") states: > > Noncontiguous subarrays may appear. It is implementation-specific > whether noncontiguous regions are updated by using one transfer for > each contiguous subregion, or whether the non-contiguous data is > packed, transferred once, and unpacked, or whether one or more larger > subarrays (no larger than the smallest contiguous region that contains > the specified subarray) are updated. > > This patch relaxes some conditions in the Fortran front-end so that > strided accesses are permitted for update directives. > > Tested with offloading to AMD GCN. OK for mainline? > > Thanks, > > Julian > > 2020-02-02 Julian Brown <julian@codesourcery.com> > > gcc/fortran/ > * openmp.c (resolve_omp_clauses): Omit OpenACC update in > contiguity check and stride-specified error. > > gcc/testsuite/ > * gfortran.dg/goacc/array-with-dt-2.f90: New test. > --- > gcc/fortran/openmp.c | 5 +++-- > gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 | 10 ++++++++++ > 2 files changed, 13 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 > > diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c > index 9a3a8f63b5e..1c8c5315329 100644 > --- a/gcc/fortran/openmp.c > +++ b/gcc/fortran/openmp.c > @@ -5192,7 +5192,8 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, > array isn't contiguous. An expression such as > arr(-n:n,-n:n) could be contiguous even if it looks > like it may not be. */ > - if (list != OMP_LIST_CACHE > + if (code->op != EXEC_OACC_UPDATE > + && list != OMP_LIST_CACHE > && list != OMP_LIST_DEPEND > && !gfc_is_simply_contiguous (n->expr, false, true) > && gfc_is_not_contiguous (n->expr)) > @@ -5224,7 +5225,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, > int i; > gfc_array_ref *ar = &array_ref->u.ar; > for (i = 0; i < ar->dimen; i++) > - if (ar->stride[i]) > + if (ar->stride[i] && code->op != EXEC_OACC_UPDATE) > { > gfc_error ("Stride should not be specified for " > "array section in %s clause at %L", > diff --git a/gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 b/gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 > new file mode 100644 > index 00000000000..807580d75a9 > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 > @@ -0,0 +1,10 @@ > +type t > + integer, allocatable :: A(:,:) > +end type t > + > +type(t), allocatable :: b(:) > + > +!$acc update host(b(::2)) > +!$acc update host(b(1)%A(::3,::4)) > +end > + ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
LGTM. However, I'd feel better if there were a testcase, which actually checks that it works correctly. (I think that means a libgomp/testsuite/ run test.) Tobias On 02.02.21 14:28, Julian Brown wrote: > OpenACC 3.0 ("2.14.4. Update Directive") states: > > Noncontiguous subarrays may appear. It is implementation-specific > whether noncontiguous regions are updated by using one transfer for > each contiguous subregion, or whether the non-contiguous data is > packed, transferred once, and unpacked, or whether one or more larger > subarrays (no larger than the smallest contiguous region that contains > the specified subarray) are updated. > > This patch relaxes some conditions in the Fortran front-end so that > strided accesses are permitted for update directives. > > Tested with offloading to AMD GCN. OK for mainline? > > Thanks, > > Julian > > 2020-02-02 Julian Brown <julian@codesourcery.com> > > gcc/fortran/ > * openmp.c (resolve_omp_clauses): Omit OpenACC update in > contiguity check and stride-specified error. > > gcc/testsuite/ > * gfortran.dg/goacc/array-with-dt-2.f90: New test. > --- > gcc/fortran/openmp.c | 5 +++-- > gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 | 10 ++++++++++ > 2 files changed, 13 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 > > diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c > index 9a3a8f63b5e..1c8c5315329 100644 > --- a/gcc/fortran/openmp.c > +++ b/gcc/fortran/openmp.c > @@ -5192,7 +5192,8 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, > array isn't contiguous. An expression such as > arr(-n:n,-n:n) could be contiguous even if it looks > like it may not be. */ > - if (list != OMP_LIST_CACHE > + if (code->op != EXEC_OACC_UPDATE > + && list != OMP_LIST_CACHE > && list != OMP_LIST_DEPEND > && !gfc_is_simply_contiguous (n->expr, false, true) > && gfc_is_not_contiguous (n->expr)) > @@ -5224,7 +5225,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, > int i; > gfc_array_ref *ar = &array_ref->u.ar; > for (i = 0; i < ar->dimen; i++) > - if (ar->stride[i]) > + if (ar->stride[i] && code->op != EXEC_OACC_UPDATE) > { > gfc_error ("Stride should not be specified for " > "array section in %s clause at %L", > diff --git a/gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 b/gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 > new file mode 100644 > index 00000000000..807580d75a9 > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 > @@ -0,0 +1,10 @@ > +type t > + integer, allocatable :: A(:,:) > +end type t > + > +type(t), allocatable :: b(:) > + > +!$acc update host(b(::2)) > +!$acc update host(b(1)%A(::3,::4)) > +end > + ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
On Thu, 4 Feb 2021 16:03:44 +0100 Tobias Burnus <tobias@codesourcery.com> wrote: > LGTM. > > However, I'd feel better if there were a testcase, which actually > checks that it works correctly. (I think that means a > libgomp/testsuite/ run test.) Thanks! FYI, I've committed this version with an additional runtime test. Julian
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c index 9a3a8f63b5e..1c8c5315329 100644 --- a/gcc/fortran/openmp.c +++ b/gcc/fortran/openmp.c @@ -5192,7 +5192,8 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, array isn't contiguous. An expression such as arr(-n:n,-n:n) could be contiguous even if it looks like it may not be. */ - if (list != OMP_LIST_CACHE + if (code->op != EXEC_OACC_UPDATE + && list != OMP_LIST_CACHE && list != OMP_LIST_DEPEND && !gfc_is_simply_contiguous (n->expr, false, true) && gfc_is_not_contiguous (n->expr)) @@ -5224,7 +5225,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, int i; gfc_array_ref *ar = &array_ref->u.ar; for (i = 0; i < ar->dimen; i++) - if (ar->stride[i]) + if (ar->stride[i] && code->op != EXEC_OACC_UPDATE) { gfc_error ("Stride should not be specified for " "array section in %s clause at %L", diff --git a/gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 b/gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 new file mode 100644 index 00000000000..807580d75a9 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 @@ -0,0 +1,10 @@ +type t + integer, allocatable :: A(:,:) +end type t + +type(t), allocatable :: b(:) + +!$acc update host(b(::2)) +!$acc update host(b(1)%A(::3,::4)) +end +
OpenACC 3.0 ("2.14.4. Update Directive") states: Noncontiguous subarrays may appear. It is implementation-specific whether noncontiguous regions are updated by using one transfer for each contiguous subregion, or whether the non-contiguous data is packed, transferred once, and unpacked, or whether one or more larger subarrays (no larger than the smallest contiguous region that contains the specified subarray) are updated. This patch relaxes some conditions in the Fortran front-end so that strided accesses are permitted for update directives. Tested with offloading to AMD GCN. OK for mainline? Thanks, Julian 2020-02-02 Julian Brown <julian@codesourcery.com> gcc/fortran/ * openmp.c (resolve_omp_clauses): Omit OpenACC update in contiguity check and stride-specified error. gcc/testsuite/ * gfortran.dg/goacc/array-with-dt-2.f90: New test. --- gcc/fortran/openmp.c | 5 +++-- gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90 | 10 ++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/goacc/array-with-dt-2.f90