| Message ID | 2b4f59d5-be38-2814-27bb-73aa7ffb4b8f@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
On May 9, 2016 4:26:50 PM GMT+02:00, Chung-Lin Tang <cltang@codesourcery.com> wrote: >Hi, this patch resolves an ICE for Fortran when using the OpenACC >host_data directive. Actually, rather than say resolve, it's more like >adjusting the front-end to same middle-end restrictions as C/C++, >namely that we only support pointers or arrays for host_data right now. > >This patch contains a little bit of adjustments in >fortran/openmp.c:resolve_omp_clauses(), >and some testcase adjustments. This has been tested without regressions >for Fortran. > >Is this okay for trunk? > >Thanks, >Chung-Lin > >2015-05-09 Chung-Lin Tang <cltang@codesourcery.com> > > gcc/ > * fortran/openmp.c (resolve_omp_clauses): Adjust use_device clause > handling to only allow pointers and arrays. Fortran has it's own ChangeLog. The patch itself looks somewhat plausible to me, fwiw, but Jakub or a FE maintainer has the say.
Ping. On 2016/5/11 02:57 AM, Bernhard Reutner-Fischer wrote: > On May 9, 2016 4:26:50 PM GMT+02:00, Chung-Lin Tang <cltang@codesourcery.com> wrote: >> Hi, this patch resolves an ICE for Fortran when using the OpenACC >> host_data directive. Actually, rather than say resolve, it's more like >> adjusting the front-end to same middle-end restrictions as C/C++, >> namely that we only support pointers or arrays for host_data right now. >> >> This patch contains a little bit of adjustments in >> fortran/openmp.c:resolve_omp_clauses(), >> and some testcase adjustments. This has been tested without regressions >> for Fortran. >> >> Is this okay for trunk? >> >> Thanks, >> Chung-Lin >> >> 2015-05-09 Chung-Lin Tang <cltang@codesourcery.com> >> >> gcc/ >> * fortran/openmp.c (resolve_omp_clauses): Adjust use_device clause >> handling to only allow pointers and arrays. > > Fortran has it's own ChangeLog. The patch itself looks somewhat plausible to me, fwiw, but Jakub or a FE maintainer has the say. >
Ping x2 On 2016/6/7 08:03 PM, Chung-Lin Tang wrote: > Ping. > > On 2016/5/11 02:57 AM, Bernhard Reutner-Fischer wrote: >> On May 9, 2016 4:26:50 PM GMT+02:00, Chung-Lin Tang <cltang@codesourcery.com> wrote: >>> Hi, this patch resolves an ICE for Fortran when using the OpenACC >>> host_data directive. Actually, rather than say resolve, it's more like >>> adjusting the front-end to same middle-end restrictions as C/C++, >>> namely that we only support pointers or arrays for host_data right now. >>> >>> This patch contains a little bit of adjustments in >>> fortran/openmp.c:resolve_omp_clauses(), >>> and some testcase adjustments. This has been tested without regressions >>> for Fortran. >>> >>> Is this okay for trunk? >>> >>> Thanks, >>> Chung-Lin >>> >>> 2015-05-09 Chung-Lin Tang <cltang@codesourcery.com> >>> >>> gcc/ >>> * fortran/openmp.c (resolve_omp_clauses): Adjust use_device clause >>> handling to only allow pointers and arrays. >> >> Fortran has it's own ChangeLog. The patch itself looks somewhat plausible to me, fwiw, but Jakub or a FE maintainer has the say. >> >
Ping x3 On 06/21/2016 02:18 PM, Chung-Lin Tang wrote: > Ping x2 > > On 2016/6/7 08:03 PM, Chung-Lin Tang wrote: >> Ping. >> >> On 2016/5/11 02:57 AM, Bernhard Reutner-Fischer wrote: >>> On May 9, 2016 4:26:50 PM GMT+02:00, Chung-Lin Tang <cltang@codesourcery.com> wrote: >>>> Hi, this patch resolves an ICE for Fortran when using the OpenACC >>>> host_data directive. Actually, rather than say resolve, it's more like >>>> adjusting the front-end to same middle-end restrictions as C/C++, >>>> namely that we only support pointers or arrays for host_data right now. >>>> >>>> This patch contains a little bit of adjustments in >>>> fortran/openmp.c:resolve_omp_clauses(), >>>> and some testcase adjustments. This has been tested without regressions >>>> for Fortran. >>>> >>>> Is this okay for trunk? >>>> >>>> Thanks, >>>> Chung-Lin >>>> >>>> 2015-05-09 Chung-Lin Tang <cltang@codesourcery.com> >>>> >>>> gcc/ >>>> * fortran/openmp.c (resolve_omp_clauses): Adjust use_device clause >>>> handling to only allow pointers and arrays. >>> >>> Fortran has it's own ChangeLog. The patch itself looks somewhat plausible to me, fwiw, but Jakub or a FE maintainer has the say. >>> >> >
Ping x4 On 2016/7/13 7:52 PM, Chung-Lin Tang wrote: > Ping x3 > > On 06/21/2016 02:18 PM, Chung-Lin Tang wrote: >> Ping x2 >> >> On 2016/6/7 08:03 PM, Chung-Lin Tang wrote: >>> Ping. >>> >>> On 2016/5/11 02:57 AM, Bernhard Reutner-Fischer wrote: >>>> On May 9, 2016 4:26:50 PM GMT+02:00, Chung-Lin Tang <cltang@codesourcery.com> wrote: >>>>> Hi, this patch resolves an ICE for Fortran when using the OpenACC >>>>> host_data directive. Actually, rather than say resolve, it's more like >>>>> adjusting the front-end to same middle-end restrictions as C/C++, >>>>> namely that we only support pointers or arrays for host_data right now. >>>>> >>>>> This patch contains a little bit of adjustments in >>>>> fortran/openmp.c:resolve_omp_clauses(), >>>>> and some testcase adjustments. This has been tested without regressions >>>>> for Fortran. >>>>> >>>>> Is this okay for trunk? >>>>> >>>>> Thanks, >>>>> Chung-Lin >>>>> >>>>> 2015-05-09 Chung-Lin Tang <cltang@codesourcery.com> >>>>> >>>>> gcc/ >>>>> * fortran/openmp.c (resolve_omp_clauses): Adjust use_device clause >>>>> handling to only allow pointers and arrays. >>>> >>>> Fortran has it's own ChangeLog. The patch itself looks somewhat plausible to me, fwiw, but Jakub or a FE maintainer has the >>>> say. >>>> >>> >>
Hi Chung-Lin, I was ignoring your patch on the grounds that one of the omp gurus should deal with it. That says, it looks OK for trunk to me. I presume that fortran/host_data-1.f90 should have the XFAIL removed? Cheers Paul On 21 July 2016 at 11:28, Chung-Lin Tang <cltang@codesourcery.com> wrote: > Ping x4 > > On 2016/7/13 7:52 PM, Chung-Lin Tang wrote: >> Ping x3 >> >> On 06/21/2016 02:18 PM, Chung-Lin Tang wrote: >>> Ping x2 >>> >>> On 2016/6/7 08:03 PM, Chung-Lin Tang wrote: >>>> Ping. >>>> >>>> On 2016/5/11 02:57 AM, Bernhard Reutner-Fischer wrote: >>>>> On May 9, 2016 4:26:50 PM GMT+02:00, Chung-Lin Tang <cltang@codesourcery.com> wrote: >>>>>> Hi, this patch resolves an ICE for Fortran when using the OpenACC >>>>>> host_data directive. Actually, rather than say resolve, it's more like >>>>>> adjusting the front-end to same middle-end restrictions as C/C++, >>>>>> namely that we only support pointers or arrays for host_data right now. >>>>>> >>>>>> This patch contains a little bit of adjustments in >>>>>> fortran/openmp.c:resolve_omp_clauses(), >>>>>> and some testcase adjustments. This has been tested without regressions >>>>>> for Fortran. >>>>>> >>>>>> Is this okay for trunk? >>>>>> >>>>>> Thanks, >>>>>> Chung-Lin >>>>>> >>>>>> 2015-05-09 Chung-Lin Tang <cltang@codesourcery.com> >>>>>> >>>>>> gcc/ >>>>>> * fortran/openmp.c (resolve_omp_clauses): Adjust use_device clause >>>>>> handling to only allow pointers and arrays. >>>>> >>>>> Fortran has it's own ChangeLog. The patch itself looks somewhat plausible to me, fwiw, but Jakub or a FE maintainer has the >>>>> say. >>>>> >>>> >>> >
On Mon, May 09, 2016 at 10:26:50PM +0800, Chung-Lin Tang wrote: > 2015-05-09 Chung-Lin Tang <cltang@codesourcery.com> > > gcc/ > * fortran/openmp.c (resolve_omp_clauses): Adjust use_device clause > handling to only allow pointers and arrays. As has been mentioned earlier, this should go into gcc/fortran/ChangeLog without fortran/ prefix. > --- gcc/fortran/openmp.c (revision 236020) > +++ gcc/fortran/openmp.c (working copy) > @@ -3743,11 +3743,18 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_claus > && CLASS_DATA (n->sym)->attr.allocatable)) > gfc_error ("ALLOCATABLE object %qs in %s clause at %L", > n->sym->name, name, &n->where); > - if (n->sym->attr.pointer > - || (n->sym->ts.type == BT_CLASS && CLASS_DATA (n->sym) > - && CLASS_DATA (n->sym)->attr.class_pointer)) > - gfc_error ("POINTER object %qs in %s clause at %L", > - n->sym->name, name, &n->where); > + if (n->sym->attr.flavor == FL_VARIABLE > + && !n->sym->as && !n->sym->attr.pointer Better put every && on a separate line if the whole if (...) doesn't fit on a single line. > + && !n->sym->attr.cray_pointer > + && !n->sym->attr.cray_pointee) This is too ugly. I'd instead move the if after the cray pointer/pointee tests, i.e. if (n->sym->attr.cray_pointer) gfc_error (...); else if (n->sym->attr.cray_pointee) gfc_error (...); else if (n->sym->attr.flavor == FL_VARIABLE && !n->sym->as && !n->sym->attr.pointer) gfc_error (...); > + gfc_error ("%s clause variable %qs at %L is neither " > + "a pointer nor an array", name, > + n->sym->name, &n->where); Why pointer rather than POINTER ? > + if (n->sym->ts.type == BT_CLASS && CLASS_DATA (n->sym) > + && CLASS_DATA (n->sym)->attr.class_pointer) > + gfc_error ("POINTER object %qs of polymorphic type in " > + "%s clause at %L", n->sym->name, name, > + &n->where); And, doesn't this mean you emit 2 errors, the first one because it is not a pointer nor array, and the second one because it is a class with attr.class_pointer? Also put && on the next line. I think best would be to make it else if after the cray pointer/pointee checks. Jakub
Index: gcc/fortran/openmp.c =================================================================== --- gcc/fortran/openmp.c (revision 236020) +++ gcc/fortran/openmp.c (working copy) @@ -3743,11 +3743,18 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_claus && CLASS_DATA (n->sym)->attr.allocatable)) gfc_error ("ALLOCATABLE object %qs in %s clause at %L", n->sym->name, name, &n->where); - if (n->sym->attr.pointer - || (n->sym->ts.type == BT_CLASS && CLASS_DATA (n->sym) - && CLASS_DATA (n->sym)->attr.class_pointer)) - gfc_error ("POINTER object %qs in %s clause at %L", - n->sym->name, name, &n->where); + if (n->sym->attr.flavor == FL_VARIABLE + && !n->sym->as && !n->sym->attr.pointer + && !n->sym->attr.cray_pointer + && !n->sym->attr.cray_pointee) + gfc_error ("%s clause variable %qs at %L is neither " + "a pointer nor an array", name, + n->sym->name, &n->where); + if (n->sym->ts.type == BT_CLASS && CLASS_DATA (n->sym) + && CLASS_DATA (n->sym)->attr.class_pointer) + gfc_error ("POINTER object %qs of polymorphic type in " + "%s clause at %L", n->sym->name, name, + &n->where); if (n->sym->attr.cray_pointer) gfc_error ("Cray pointer object %qs in %s clause at %L", n->sym->name, name, &n->where); Index: gcc/testsuite/gfortran.dg/goacc/uninit-use-device-clause.f95 =================================================================== --- gcc/testsuite/gfortran.dg/goacc/uninit-use-device-clause.f95 (revision 236020) +++ gcc/testsuite/gfortran.dg/goacc/uninit-use-device-clause.f95 (working copy) @@ -2,9 +2,9 @@ ! { dg-additional-options "-Wuninitialized" } subroutine test - integer :: i + integer, pointer :: p - !$acc host_data use_device(i) ! { dg-warning "is used uninitialized in this function" } + !$acc host_data use_device(p) ! { dg-warning "is used uninitialized in this function" } !$acc end host_data end subroutine test Index: gcc/testsuite/gfortran.dg/goacc/list.f95 =================================================================== --- gcc/testsuite/gfortran.dg/goacc/list.f95 (revision 236020) +++ gcc/testsuite/gfortran.dg/goacc/list.f95 (working copy) @@ -76,19 +76,19 @@ program test !$acc parallel private (i) firstprivate (i) ! { dg-error "present on multiple clauses" } !$acc end parallel - !$acc host_data use_device(i) + !$acc host_data use_device(i) ! { dg-error "neither a pointer nor an array" } !$acc end host_data - !$acc host_data use_device(c, d) + !$acc host_data use_device(c, d) ! { dg-error "neither a pointer nor an array" } !$acc end host_data !$acc host_data use_device(a) !$acc end host_data - !$acc host_data use_device(i, j, k, l, a) + !$acc host_data use_device(i, j, k, l, a) ! { dg-error "neither a pointer nor an array" } !$acc end host_data - !$acc host_data use_device (i) use_device (j) + !$acc host_data use_device (i) use_device (j) ! { dg-error "neither a pointer nor an array" } !$acc end host_data !$acc host_data use_device ! { dg-error "Unclassifiable OpenACC directive" } @@ -99,13 +99,17 @@ program test !$acc host_data use_device(10) ! { dg-error "Syntax error" } - !$acc host_data use_device(/b/, /b/) ! { dg-error "present on multiple clauses" } + !$acc host_data use_device(/b/, /b/) !$acc end host_data + ! { dg-error "neither a pointer nor an array" "" { target *-*-* } 102 } + ! { dg-error "present on multiple clauses" "" { target *-*-* } 102 } - !$acc host_data use_device(i, j, i) ! { dg-error "present on multiple clauses" } + !$acc host_data use_device(i, j, i) !$acc end host_data + ! { dg-error "neither a pointer nor an array" "" { target *-*-* } 107 } + ! { dg-error "present on multiple clauses" "" { target *-*-* } 107 } - !$acc host_data use_device(p1) ! { dg-error "POINTER" } + !$acc host_data use_device(p1) !$acc end host_data end program test Index: gcc/testsuite/gfortran.dg/goacc/host_data-tree.f95 =================================================================== --- gcc/testsuite/gfortran.dg/goacc/host_data-tree.f95 (revision 236020) +++ gcc/testsuite/gfortran.dg/goacc/host_data-tree.f95 (working copy) @@ -3,9 +3,9 @@ program test implicit none - integer :: i = 1 + integer, pointer :: p - !$acc host_data use_device(i) + !$acc host_data use_device(p) !$acc end host_data end program test -! { dg-final { scan-tree-dump-times "pragma acc host_data use_device_ptr\\(i\\)" 1 "original" } } +! { dg-final { scan-tree-dump-times "pragma acc host_data use_device_ptr\\(p\\)" 1 "original" } } Index: libgomp/testsuite/libgomp.oacc-fortran/host_data-1.f90 =================================================================== --- libgomp/testsuite/libgomp.oacc-fortran/host_data-1.f90 (revision 0) +++ libgomp/testsuite/libgomp.oacc-fortran/host_data-1.f90 (revision 0) @@ -0,0 +1,35 @@ +! { dg-do run } +! { dg-additional-options "-cpp" } + +! { dg-xfail-if "TODO" { *-*-* } } +! { dg-excess-errors "TODO" } + +program test + implicit none + + integer, target :: i, arr(1000) + integer, pointer :: ip, iph + integer, contiguous, pointer :: parr(:), parrh(:) + + ! Assign the same targets + ip => i + parr => arr + iph => i + parrh => arr + + !$acc data copyin(i, arr) + !$acc host_data use_device(ip, parr) + + ! Test how the pointers compare inside a host_data construct +#if ACC_MEM_SHARED + if (.not. associated(ip, iph)) call abort + if (.not. associated(parr, parrh)) call abort +#else + if (associated(ip, iph)) call abort + if (associated(parr, parrh)) call abort +#endif + + !$acc end host_data + !$acc end data + +end program test