Hi Tobias,
Thanks for review! Comments below.
On Tue, 2 Feb 2021 17:32:03 +0100
Tobias Burnus <tob...@codesourcery.com> wrote:
> > diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
> > index 00358ca4d39..8d8da4593c3 100644
> > --- a/gcc/fortran/trans-openmp.c
> > +++ b/gcc/fortran/trans-openmp.c
> > @@ -3013,7 +3013,8 @@ gfc_trans_omp_clauses (stmtblock_t *block,
> > gfc_omp_clauses *clauses, }
> >
> > OMP_CLAUSE_DECL (node)
> > - = build_fold_indirect_ref (data);
> > + = (POINTER_TYPE_P (TREE_TYPE (data))
> > + ? build_fold_indirect_ref (data) :
> > data); OMP_CLAUSE_SIZE (node) = size;
> > node2 = build_omp_clause
> > (input_location, OMP_CLAUSE_MAP);
>
> I am a bit surprised given that:
> if (sym_attr.pointer || (openacc &&
> sym_attr.allocatable))
>
> I wonder whether we have a size problem:
> data = inner;
> size = TYPE_SIZE_UNIT (TREE_TYPE
> (inner));
I think the code was correct wrt. data sizes, but the logic was
admittedly rather convoluted. I think the attached is better -- the
essential "weirdness" of the previous patch is that it seemed like a
BT_DERIVED-typed "inner" could either be a pointer or not, and likewise
for the BT_CLASS case.
Actually though, that wasn't true. All the non-POINTER_TYPE_P cases
involve BT_DERIVED, because the decl is transparently dereferenced
by the earlier gfc_conv_component_ref call. BT_CLASS pointers, however,
were not dereferenced -- so "data" appeared as an actual pointer. The
pre-patch code actually only worked in the BT_CLASS case.
So, I think the new version makes more sense. (The "size" field is
either transparently dereferenced via gfc_conv_component_ref, or comes
from the class's vtable, so it's never the size of the non-dereferenced
pointer.)
Re-tested with offloading to AMD GCN. OK?
Thank you,
Julian
>From 158f733899caa59c1cb9f53a14fa0544ae3e8878 Mon Sep 17 00:00:00 2001
From: Julian Brown <jul...@codesourcery.com>
Date: Fri, 29 Jan 2021 15:37:27 -0800
Subject: [PATCH] openacc: Dereference BT_CLASS data pointers but not
BT_DERIVED pointers
The stanza in gfc_trans_omp_clauses that handles derived type members
that are themselves derived type pointers or class pointers now adds
an explicit dereference only for the latter. The former is already
dereferenced transparently in gfc_conv_component_ref.
gcc/fortran/
* trans-openmp.c (gfc_trans_omp_clauses): Fix dereferencing for
BT_DERIVED members.
gcc/testsuite/
* gfortran.dg/goacc/derived-classtypes-1.f95: New test.
---
gcc/fortran/trans-openmp.c | 7 +-
.../goacc/derived-classtypes-1.f95 | 129 ++++++++++++++++++
2 files changed, 133 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/goacc/derived-classtypes-1.f95
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 00358ca4d39..a5fe0e76af2 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -3004,6 +3004,8 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses,
if (lastcomp->u.c.component->ts.type == BT_CLASS)
{
data = gfc_class_data_get (inner);
+ gcc_assert (POINTER_TYPE_P (TREE_TYPE (data)));
+ data = build_fold_indirect_ref (data);
size = gfc_class_vtab_size_get (inner);
}
else /* BT_DERIVED. */
@@ -3012,8 +3014,7 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses,
size = TYPE_SIZE_UNIT (TREE_TYPE (inner));
}
- OMP_CLAUSE_DECL (node)
- = build_fold_indirect_ref (data);
+ OMP_CLAUSE_DECL (node) = data;
OMP_CLAUSE_SIZE (node) = size;
node2 = build_omp_clause (input_location,
OMP_CLAUSE_MAP);
@@ -3021,7 +3022,7 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses,
openacc
? GOMP_MAP_ATTACH_DETACH
: GOMP_MAP_ALWAYS_POINTER);
- OMP_CLAUSE_DECL (node2) = data;
+ OMP_CLAUSE_DECL (node2) = build_fold_addr_expr (data);
OMP_CLAUSE_SIZE (node2) = size_int (0);
}
else
diff --git a/gcc/testsuite/gfortran.dg/goacc/derived-classtypes-1.f95 b/gcc/testsuite/gfortran.dg/goacc/derived-classtypes-1.f95
new file mode 100644
index 00000000000..e6cf09c6d3c
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/derived-classtypes-1.f95
@@ -0,0 +1,129 @@
+type :: type1
+ integer :: a
+end type type1
+
+type :: type2
+ integer, pointer :: b
+end type type2
+
+type :: aux1
+ integer :: y
+end type aux1
+
+type, extends(aux1) :: aux
+ integer :: x
+end type aux
+
+type :: type3
+ class(aux), pointer :: c(:)
+end type type3
+
+type :: type4
+ integer, pointer :: d(:)
+end type type4
+
+type :: type5
+ type(aux) :: e
+end type type5
+
+type :: type6
+ type(aux), pointer :: f
+end type type6
+
+type :: type7
+ class(aux), pointer :: g
+end type type7
+
+type(type1) :: foo
+type(type2) :: bar
+type(type3) :: qux
+type(type4) :: quux
+type(type5) :: fred
+type(type6) :: jim
+type(type7) :: shiela
+
+type(type1), pointer :: pfoo
+type(type2), pointer :: pbar
+type(type3), pointer :: pqux
+type(type4), pointer :: pquux
+type(type5), pointer :: pfred
+type(type6), pointer :: pjim
+type(type7), pointer :: pshiela
+
+class(type1), pointer :: cfoo
+class(type2), pointer :: cbar
+class(type3), pointer :: cqux
+class(type4), pointer :: cquux
+class(type5), pointer :: cfred
+class(type6), pointer :: cjim
+class(type7), pointer :: cshiela
+
+class(type1), allocatable :: acfoo
+class(type2), allocatable :: acbar
+class(type3), allocatable :: acqux
+class(type4), allocatable :: acquux
+class(type5), allocatable :: acfred
+class(type6), allocatable :: acjim
+class(type7), allocatable :: acshiela
+
+!$acc enter data copyin(foo)
+!$acc enter data copyin(foo%a)
+!$acc enter data copyin(bar)
+!$acc enter data copyin(bar%b)
+!$acc enter data copyin(qux)
+!!$acc enter data copyin(qux%c)
+!$acc enter data copyin(quux)
+!$acc enter data copyin(quux%d)
+!$acc enter data copyin(fred)
+!$acc enter data copyin(fred%e)
+!$acc enter data copyin(jim)
+!$acc enter data copyin(jim%f)
+!$acc enter data copyin(shiela)
+!$acc enter data copyin(shiela%g)
+
+!$acc enter data copyin(pfoo)
+!$acc enter data copyin(pfoo%a)
+!$acc enter data copyin(pbar)
+!$acc enter data copyin(pbar%b)
+!$acc enter data copyin(pqux)
+!!$acc enter data copyin(pqux%c)
+!$acc enter data copyin(pquux)
+!$acc enter data copyin(pquux%d)
+!$acc enter data copyin(pfred)
+!$acc enter data copyin(pfred%e)
+!$acc enter data copyin(pjim)
+!$acc enter data copyin(pjim%f)
+!$acc enter data copyin(pshiela)
+!$acc enter data copyin(pshiela%g)
+
+!$acc enter data copyin(cfoo)
+!$acc enter data copyin(cfoo%a)
+!$acc enter data copyin(cbar)
+!$acc enter data copyin(cbar%b)
+!$acc enter data copyin(cqux)
+!!$acc enter data copyin(cqux%c)
+!$acc enter data copyin(cquux)
+!$acc enter data copyin(cquux%d)
+!$acc enter data copyin(cfred)
+!$acc enter data copyin(cfred%e)
+!$acc enter data copyin(cjim)
+!$acc enter data copyin(cjim%f)
+!$acc enter data copyin(cshiela)
+!$acc enter data copyin(cshiela%g)
+
+!$acc enter data copyin(acfoo)
+!$acc enter data copyin(acfoo%a)
+!$acc enter data copyin(acbar)
+!$acc enter data copyin(acbar%b)
+!$acc enter data copyin(acqux)
+!!$acc enter data copyin(acqux%c)
+!$acc enter data copyin(acquux)
+!$acc enter data copyin(acquux%d)
+!$acc enter data copyin(acfred)
+!$acc enter data copyin(acfred%e)
+!$acc enter data copyin(acjim)
+!$acc enter data copyin(acjim%f)
+!$acc enter data copyin(acshiela)
+!$acc enter data copyin(acshiela%g)
+
+end
--
2.29.2
