On 5/14/21 11:26 PM, Julian Brown wrote:
> This patch reworks indirect struct handling in gimplify.c (i.e. for struct
> components mapped with "mystruct->a[0:n]", "mystruct->b", etc.), for
> both OpenACC and OpenMP. The key observation leading to these changes
> was that component mappings of references-to-structures is already
> implemented and working, and indirect struct component handling via a
> pointer can work quite similarly. That lets us remove some earlier,
> special-case handling for mapping indirect struct component accesses
> for OpenACC, which required the pointed-to struct to be manually mapped
> before the indirect component mapping.
>
> With this patch, you can map struct components directly (e.g. an array
> slice "mystruct->a[0:n]") just like you can map a non-indirect struct
> component slice ("mystruct.a[0:n]"). Both references-to-pointers (with
> the former syntax) and references to structs (with the latter syntax)
> work now.
>
> For Fortran class pointers, we no longer re-use GOMP_MAP_TO_PSET for the
> class metadata (the structure that points to the class data and vptr)
> -- it is instead treated as any other struct.
>
> For C++, the struct handling also works for class members ("this->foo"),
> without having to explicitly map "this[:1]" first.
>
> For OpenACC, we permit chained indirect component references
> ("mystruct->a->b[0:n]"), though only the last part of such mappings will
> trigger an attach/detach operation. To properly use such a construct
> on the target, you must still manually map "mystruct->a[:1]" first --
> but there's no need to map "mystruct[:1]" explicitly before that.
>
> This patch incorporates parts of Chung-Lin's patch "Recommit "Enable
> gimplify GOMP_MAP_STRUCT handling of (COMPONENT_REF (INDIRECT_REF
> ...)) map clauses"." from the og10 branch.
>
> OK for trunk?
>
> Thanks,
>
> Julian
>
> 2021-05-14 Julian Brown <jul...@codesourcery.com>
> Chung-Lin Tang <clt...@codesourcery.com>
>
> gcc/fortran/
> * trans-openmp.c (gfc_trans_omp_clauses): Don't create GOMP_MAP_TO_PSET
> mappings for class metadata, nor GOMP_MAP_POINTER mappings for
> POINTER_TYPE_P decls.
>
> gcc/
> * gimplify.c (tree-hash-traits.h): Include.
> (extract_base_bit_offset): Add BASE_IND parameter. Handle
> pointer-typed indirect references alongside reference-typed ones.
> (strip_components_and_deref, aggregate_base_p): New functions.
> (build_struct_group): Update struct_map_to_clause type. Add pointer
> type indirect ref handling, including chained references. Handle
> pointers and references to structs in OpenACC regions as well as
> OpenMP ones.
> (gimplify_scan_omp_clauses): Remove struct_deref_set handling. Rework
> pointer-type indirect structure access handling to work more like
> the reference-typed handling.
> * omp-low.c (scan_sharing_clauses): Handle pointer-type indirect struct
> references, and references to pointers to structs also.
>
> gcc/testsuite/
> * g++.dg/goacc/member-array-acc.C: New test (XFAILed for now).
> * g++.dg/gomp/member-array-omp.C: New test (XFAILed for now).
>
> libgomp/
> * testsuite/libgomp.oacc-c-c++-common/deep-copy-15.c: New test.
> * testsuite/libgomp.oacc-c-c++-common/deep-copy-16.c: New test.
> * testsuite/libgomp.oacc-c++/deep-copy-17.C: New test.
> ---
> gcc/fortran/trans-openmp.c | 20 +-
> gcc/gimplify.c | 285 ++++++++++--------
> gcc/omp-low.c | 16 +-
> gcc/testsuite/g++.dg/goacc/member-array-acc.C | 14 +
> gcc/testsuite/g++.dg/gomp/member-array-omp.C | 14 +
> .../testsuite/libgomp.oacc-c++/deep-copy-17.C | 101 +++++++
> .../libgomp.oacc-c-c++-common/deep-copy-15.c | 71 +++++
> .../libgomp.oacc-c-c++-common/deep-copy-16.c | 231 ++++++++++++++
> 8 files changed, 612 insertions(+), 140 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/goacc/member-array-acc.C
> create mode 100644 gcc/testsuite/g++.dg/gomp/member-array-omp.C
> create mode 100644 libgomp/testsuite/libgomp.oacc-c++/deep-copy-17.C
> create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-15.c
> create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-16.c
>
> diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
> index 5666cd68c7e..ff614ffe744 100644
> --- a/gcc/fortran/trans-openmp.c
> +++ b/gcc/fortran/trans-openmp.c
> @@ -2721,30 +2721,16 @@ gfc_trans_omp_clauses (stmtblock_t *block,
> gfc_omp_clauses *clauses,
> tree present = gfc_omp_check_optional_argument (decl, true);
> if (openacc && n->sym->ts.type == BT_CLASS)
> {
> - tree type = TREE_TYPE (decl);
> if (n->sym->attr.optional)
> sorry ("optional class parameter");
> - if (POINTER_TYPE_P (type))
> - {
> - node4 = build_omp_clause (input_location,
> - OMP_CLAUSE_MAP);
> - OMP_CLAUSE_SET_MAP_KIND (node4, GOMP_MAP_POINTER);
> - OMP_CLAUSE_DECL (node4) = decl;
> - OMP_CLAUSE_SIZE (node4) = size_int (0);
> - decl = build_fold_indirect_ref (decl);
> - }
> tree ptr = gfc_class_data_get (decl);
> ptr = build_fold_indirect_ref (ptr);
> OMP_CLAUSE_DECL (node) = ptr;
> OMP_CLAUSE_SIZE (node) = gfc_class_vtab_size_get (decl);
> node2 = build_omp_clause (input_location, OMP_CLAUSE_MAP);
> - OMP_CLAUSE_SET_MAP_KIND (node2, GOMP_MAP_TO_PSET);
> - OMP_CLAUSE_DECL (node2) = decl;
> - OMP_CLAUSE_SIZE (node2) = TYPE_SIZE_UNIT (type);
> - node3 = build_omp_clause (input_location, OMP_CLAUSE_MAP);
> - OMP_CLAUSE_SET_MAP_KIND (node3, GOMP_MAP_ATTACH_DETACH);
> - OMP_CLAUSE_DECL (node3) = gfc_class_data_get (decl);
> - OMP_CLAUSE_SIZE (node3) = size_int (0);
> + OMP_CLAUSE_SET_MAP_KIND (node2, GOMP_MAP_ATTACH_DETACH);
> + OMP_CLAUSE_DECL (node2) = gfc_class_data_get (decl);
> + OMP_CLAUSE_SIZE (node2) = size_int (0);
> goto finalize_map_clause;
> }
> else if (POINTER_TYPE_P (TREE_TYPE (decl))
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 69ab637367c..c0f068a725d 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -53,6 +53,7 @@ along with GCC; see the file COPYING3. If not see
> #include "langhooks.h"
> #include "tree-cfg.h"
> #include "tree-ssa.h"
> +#include "tree-hash-traits.h"
> #include "omp-general.h"
> #include "omp-low.h"
> #include "gimple-low.h"
> @@ -8351,8 +8352,8 @@ build_struct_comp_nodes (enum tree_code code, tree
> grp_start, tree grp_end,
> has array type, else return NULL. */
>
> static tree
> -extract_base_bit_offset (tree base, tree *base_ref, poly_int64 *bitposp,
> - poly_offset_int *poffsetp)
> +extract_base_bit_offset (tree base, tree *base_ind, tree *base_ref,
> + poly_int64 *bitposp, poly_offset_int *poffsetp)
> {
> tree offset;
> poly_int64 bitsize, bitpos;
> @@ -8360,6 +8361,9 @@ extract_base_bit_offset (tree base, tree *base_ref,
> poly_int64 *bitposp,
> int unsignedp, reversep, volatilep = 0;
> poly_offset_int poffset;
>
> + if (base_ind)
> + *base_ind = NULL_TREE;
> +
> if (base_ref)
> *base_ref = NULL_TREE;
>
> @@ -8380,14 +8384,27 @@ extract_base_bit_offset (tree base, tree *base_ref,
> poly_int64 *bitposp,
> base = get_inner_reference (base, &bitsize, &bitpos, &offset, &mode,
> &unsignedp, &reversep, &volatilep);
>
> - tree orig_base = base;
> -
> + if ((TREE_CODE (base) == INDIRECT_REF
> + || (TREE_CODE (base) == MEM_REF
> + && integer_zerop (TREE_OPERAND (base, 1))))
> + && TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0))) == POINTER_TYPE)
> + {
> + if (base_ind)
> + *base_ind = base;
> + base = TREE_OPERAND (base, 0);
> + }
> if ((TREE_CODE (base) == INDIRECT_REF
> || (TREE_CODE (base) == MEM_REF
> && integer_zerop (TREE_OPERAND (base, 1))))
> && DECL_P (TREE_OPERAND (base, 0))
> && TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0))) == REFERENCE_TYPE)
> - base = TREE_OPERAND (base, 0);
> + {
> + if (base_ref)
> + *base_ref = base;
> + base = TREE_OPERAND (base, 0);
> + }
> +
> + STRIP_NOPS (base);
>
> gcc_assert (offset == NULL_TREE || poly_int_tree_p (offset));
>
> @@ -8402,10 +8419,6 @@ extract_base_bit_offset (tree base, tree *base_ref,
> poly_int64 *bitposp,
> *bitposp = bitpos;
> *poffsetp = poffset;
>
> - /* Set *BASE_REF if BASE was a dereferenced reference variable. */
> - if (base_ref && orig_base != base)
> - *base_ref = orig_base;
> -
> return base;
> }
>
> @@ -8422,6 +8435,48 @@ is_or_contains_p (tree expr, tree base_ptr)
> return expr == base_ptr;
> }
>
> +/* Remove COMPONENT_REFS and indirections from EXPR. */
> +
> +static tree
> +strip_components_and_deref (tree expr)
> +{
> + while (TREE_CODE (expr) == COMPONENT_REF
> + || TREE_CODE (expr) == INDIRECT_REF
> + || (TREE_CODE (expr) == MEM_REF
> + && integer_zerop (TREE_OPERAND (expr, 1))))
> + expr = TREE_OPERAND (expr, 0);
> +
> + return expr;
> +}
> +
> +/* Return TRUE if EXPR is something we will use as the base of an aggregate
> + access, either:
> +
> + - a DECL_P.
> + - a struct component with no indirection ("a.b.c").
> + - a struct component with indirection ("a->b->c").
> +*/
> +
> +static bool
> +aggregate_base_p (tree expr)
> +{
> + while (TREE_CODE (expr) == COMPONENT_REF
> + && (DECL_P (TREE_OPERAND (expr, 0))
> + || (TREE_CODE (TREE_OPERAND (expr, 0)) == COMPONENT_REF)))
> + expr = TREE_OPERAND (expr, 0);
> +
> + if (DECL_P (expr))
> + return true;
> +
> + if (TREE_CODE (expr) == COMPONENT_REF
> + && (TREE_CODE (TREE_OPERAND (expr, 0)) == INDIRECT_REF
> + || (TREE_CODE (TREE_OPERAND (expr, 0)) == MEM_REF
> + && integer_zerop (TREE_OPERAND (TREE_OPERAND (expr, 0), 1)))))
> + return true;
> +
> + return false;
> +}
> +
> /* Implement OpenMP 5.x map ordering rules for target directives. There are
> several rules, and with some level of ambiguity, hopefully we can at least
> collect the complexity here in one place. */
> @@ -8715,19 +8770,26 @@ static tree
> build_struct_group (struct gimplify_omp_ctx *ctx,
> enum omp_region_type region_type, enum tree_code code,
> tree decl, unsigned int *flags, tree c,
> - hash_map<tree, tree> *&struct_map_to_clause,
> + hash_map<tree_operand_hash, tree> *&struct_map_to_clause,
> tree *&prev_list_p, tree *&list_p, bool *cont)
> {
> poly_offset_int coffset;
> poly_int64 cbitpos;
> - tree base_ref;
> + tree base_ind, base_ref;
> + tree *list_in_p = list_p, *prev_list_in_p = prev_list_p;
>
Is this a kind of debug code?
This fails to compile:
../../gcc-trunk/gcc/gimplify.c: In function ‘tree_node*
build_struct_group(gimplify_omp_ctx*, omp_region_type, tree_code, tree,
unsigned int*, tree, hash_map<tree_operand_hash, tree_node*>*&, tree_node**&,
tree_node**&, bool*)’:
../../gcc-trunk/gcc/gimplify.c:8779:9: error: unused variable ‘list_in_p’
[-Werror=unused-variable]
8779 | tree *list_in_p = list_p, *prev_list_in_p = prev_list_p;
| ^~~~~~~~~
../../gcc-trunk/gcc/gimplify.c:8779:30: error: unused variable ‘prev_list_in_p’
[-Werror=unused-variable]
8779 | tree *list_in_p = list_p, *prev_list_in_p = prev_list_p;
| ^~~~~~~~~~~~~~
You need to boot-strap and do your regression testing before sending
a patch to this list.
Thanks
Bernd.
