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 +