[PATCH] OpenACC 2.6 manual deep copy support (attach/detach)
Julian Brown
julian@codesourcery.com
Wed Nov 6 18:44:00 GMT 2019
Hi,
This version of the patch has been adjusted for changes to the
reference-count overhaul patch last posted here:
https://gcc.gnu.org/ml/gcc-patches/2019-10/msg02041.html
In particular, it incorporates the idea alluded to in that patch
relating to adding the new "attach_count" field to the memory-mapping
splay tree key type without growing that structure in the common case
(i.e. when structure components are not being mapped, or for OpenMP).
In short, a new auxiliary structure is added containing the previous
"link_key" and "attach_count" fields: so, you can either have both
pointers (though of course one of them may be NULL), or in the common
case no aux pointer at all, so no growth in the base struct size.
I've also addressed a couple of Thomas's earlier review comments from
December 2018:
https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01620.html
And also, as a minor point, the patch reincorporates the patch adding
c_omp_map_clause_name, since it refers to the newly-added
GOMP_MAP_ATTACH values so the two patches are mutually dependent. That
one was posted here:
https://gcc.gnu.org/ml/gcc-patches/2019-10/msg00443.html
On Fri, 18 Oct 2019 18:47:08 +0200
Thomas Schwinge <thomas@codesourcery.com> wrote:
> Hi!
>
> While reviewing
> <http://mid.mail-archive.com/20191003163505.49997-2-julian@codesourcery.com>
> "OpenACC reference count overhaul", I've just now stumbled over one
> thing that originally was designed here:
>
> On 2018-12-10T19:41:37+0000, Julian Brown <julian@codesourcery.com>
> wrote:
> > On Fri, 7 Dec 2018 14:50:19 +0100
> > Jakub Jelinek <jakub@redhat.com> wrote:
> >
> >> On Fri, Nov 30, 2018 at 03:41:09AM -0800, Julian Brown wrote:
> >> > @@ -918,8 +920,13 @@ struct splay_tree_key_s {
> >> > uintptr_t tgt_offset;
> >> > /* Reference count. */
> >> > uintptr_t refcount;
> >> > - /* Dynamic reference count. */
> >> > - uintptr_t dynamic_refcount;
> >> > + /* Reference counts beyond those that represent genuine
> >> > references in the
> >> > + linked splay tree key/target memory structures, e.g. for
> >> > multiple OpenACC
> >> > + "present increment" operations (via "acc enter data")
> >> > refering to the same
> >> > + host-memory block. */
> >> > + uintptr_t virtual_refcount;
> >> > + /* For a block with attached pointers, the attachment
> >> > counters for each. */
> >> > + unsigned short *attach_count;
> >> > /* Pointer to the original mapping of "omp declare target
> >> > link" object. */ splay_tree_key link_key;
> >> > };
> >>
> >> This is something I'm worried about a lot, the nodes keep growing
> >> way too much.
>
> Is that just a would-be-nice-to-avoid, or is it an actual problem?
>
> If the latter, can we maybe move some data into on-the-side data
> structures, say an associative array keyed by [something suitable]? I
> would assume that compared to actual host to/from device data movement
> (or even lookup etc.), lookup of values from such an associative array
> should be relatively cheap?
I'd be extremely wary of adding a completely separate off-the-side
structure to keep track of attachment counters: the reference-counting
behaviour is already complicated enough, and the risk of messing things
up with another indirectly-linked structure to keep track of is too
high (never mind the extra runtime overhead). With the approach in this
patch, at least the extra info for link_key/attach_count is directly
accessible from the splay tree key struct via pointer indirection.
This version entails slight additional overhead (another malloc'd
block and another pointer indirection) for the link_key field (and also
for the attach_count pointer). I've not benchmarked memory use or
performance though, so I'm not sure how much impact this has on real
code.
> I'm bringing this up, because:
>
> >> Is there a way to reuse some other field if it is of
> >> certain kind?
> >
> > How about this -- it seems that the link_key is only used for
> > OpenMP,
>
> So, is that actually correct? Per my understanding, for the OpenACC
> 'link' clause we uses 'GOMP_MAP_LINK', which sets "omp declare target
> link", and thus:
>
> > and the attach count is only needed for OpenACC. So the obvious
> > thing to do is probably to put those two together into a tagged
> > union. The question is where to put the tag?
> >
> > Options are, I guess:
> >
> > 1. The high or low bits of the address. Potentially non-portable,
> > ugly.
> >
> > 2. Or, the virtual refcount is also only needed for OpenACC, so we
> > can reserve a magic value for that field to act as a tag.
> >
> > I've tried implementing the latter in the attached patch, and it
> > seems to work OK.
>
> ... this is not actually feasible?
I'd missed that link_key could be used for OpenACC as well as OpenMP
(obviously). This version of the patch avoids that problem.
Is this version OK? Re-tested with offloading to nvptx.
Thanks,
Julian
ChangeLog
2019-11-06 Julian Brown <julian@codesourcery.com>
Thomas Schwinge <thomas@codesourcery.com>
gcc/c-family/
* c-common.h (c_omp_map_clause_name): Add prototype.
* c-omp.c (c_omp_map_clause_name): New function.
* c-pragma.h (pragma_omp_clause): Add PRAGMA_OACC_CLAUSE_ATTACH and
PRAGMA_OACC_CLAUSE_DETACH.
gcc/c/
* c-parser.c (c_parser_omp_clause_name): Add parsing of attach and
detach clauses.
(c_parser_omp_variable_list): Add ALLOW_DEREF optional parameter.
Allow deref (->) in variable lists if true.
(c_parser_omp_var_list_parens): Add ALLOW_DEREF optional parameter.
Pass to c_parser_omp_variable_list.
(c_parser_oacc_data_clause): Support attach and detach clauses. Update
call to c_parser_omp_variable_list.
(c_parser_oacc_all_clauses): Support attach and detach clauses.
(OACC_DATA_CLAUSE_MASK, OACC_ENTER_DATA_CLAUSE_MASK)
(OACC_KERNELS_CLAUSE_MASK, OACC_PARALLEL_CLAUSE_MASK): Add
PRAGMA_OACC_CLAUSE_ATTACH.
(OACC_EXIT_DATA_CLAUSE_MASK): Add PRAGMA_OACC_CLAUSE_DETACH.
* c-typeck.c (handle_omp_array_sections_1): Reject subarrays for attach
and detach. Support deref.
(handle_omp_array_sections): Use GOMP_MAP_ATTACH_DETACH instead of
GOMP_MAP_ALWAYS_POINTER for OpenACC.
(c_oacc_check_attachments): New function.
(c_finish_omp_clauses): Check attach/detach arguments for being
pointers using above. Support deref.
gcc/cp/
* parser.c (cp_parser_omp_clause_name): Support attach and detach
clauses.
(cp_parser_omp_var_list_no_open): Add ALLOW_DEREF optional parameter.
Parse deref if true.
(cp_parser_omp_var_list): Add ALLOW_DEREF optional parameter. Pass to
cp_parser_omp_var_list_no_open.
(cp_parser_oacc_data_clause): Support attach and detach clauses.
Update call to cp_parser_omp_var_list_no_open.
(cp_parser_oacc_all_clauses): Support attach and detach.
(OACC_DATA_CLAUSE_MASK, OACC_ENTER_DATA_CLAUSE_MASK)
(OACC_KERNELS_CLAUSE_MASK, OACC_PARALLEL_CLAUSE_MASK): Add
PRAGMA_OACC_CLAUSE_ATTACH.
(OACC_EXIT_DATA_CLAUSE_MASK): Add PRAGMA_OACC_CLAUSE_DETACH.
* semantics.c (handle_omp_array_sections_1): Reject subarrays for
attach and detach.
(handle_omp_array_sections): Use GOMP_MAP_ATTACH_DETACH instead of
GOMP_MAP_ALWAYS_POINTER for OpenACC.
(cp_oacc_check_attachments): New function.
(finish_omp_clauses): Use above function. Allow structure fields and
class members to appear in OpenACC data clauses. Support
GOMP_MAP_ATTACH_DETACH. Support deref.
gcc/fortran/
* gfortran.h (gfc_omp_map_op): Add OMP_MAP_ATTACH, OMP_MAP_DETACH.
* openmp.c (gfc_match_omp_variable_list): Add allow_derived parameter.
Parse derived-type member accesses if true.
(omp_mask2): Add OMP_CLAUSE_ATTACH and OMP_CLAUSE_DETACH.
(gfc_match_omp_map_clause): Add allow_derived parameter. Pass to
gfc_match_omp_variable_list.
(gfc_match_omp_clauses): Support attach and detach. Support derived
types for appropriate OpenACC directives.
(OACC_PARALLEL_CLAUSES, OACC_KERNELS_CLAUSES, OACC_DATA_CLAUSES)
(OACC_ENTER_DATA_CLAUSES): Add OMP_CLAUSE_ATTACH.
(OACC_EXIT_DATA_CLAUSES): Add OMP_CLAUSE_DETACH.
(check_symbol_not_pointer): Don't disallow pointer objects of derived
type.
(resolve_oacc_data_clauses): Don't disallow allocatable derived types.
(resolve_omp_clauses): Perform duplicate checking only for non-derived
type component accesses (plain variables and arrays or array sections).
Support component refs.
* trans-expr.c (gfc_conv_component_ref,
conv_parent_component_references): Make global.
(gfc_auto_dereference_var): New function, broken out of...
(gfc_conv_variable): ...here. Call above function.
* trans-openmp.c (gfc_omp_privatize_by_reference): Support component
refs.
(gfc_trans_omp_array_section): New function, broken out of...
(gfc_trans_omp_clauses): ...here. Support component refs/derived
types, attach and detach clauses.
* trans.h (gfc_conv_component_ref, conv_parent_component_references,
gfc_auto_dereference_var): Add prototypes.
gcc/
* gimplify.c (gimplify_omp_var_data): Add GOVD_MAP_HAS_ATTACHMENTS.
(insert_struct_comp_map): Support derived-type member mappings
for arrays with descriptors which use GOMP_MAP_TO_PSET. Support
GOMP_MAP_ATTACH_DETACH.
(gimplify_scan_omp_clauses): Tidy up OACC_ENTER_DATA/OACC_EXIT_DATA
mappings. Handle attach/detach clauses and component references.
(gimplify_adjust_omp_clauses_1): Skip adjustments for explicit
attach/detach clauses.
(gimplify_omp_target_update): Handle finalize for detach.
* omp-low.c (lower_omp_target): Support GOMP_MAP_ATTACH,
GOMP_MAP_DETACH, GOMP_MAP_FORCE_DETACH.
* tree-pretty-print.c (dump_omp_clause): Likewise, plus
GOMP_MAP_ATTACH_DETACH.
include/
* gomp-constants.h (GOMP_MAP_FLAG_SPECIAL_4, GOMP_MAP_DEEP_COPY):
Define.
(gomp_map_kind): Add GOMP_MAP_ATTACH, GOMP_MAP_DETACH,
GOMP_MAP_FORCE_DETACH.
gcc/testsuite/
* c-c++-common/goacc/deep-copy-arrayofstruct.c: New test.
* c-c++-common/goacc/mdc-1.c: New test.
* c-c++-common/goacc/mdc-2.c: New test.
* gcc.dg/goacc/mdc.C: New test.
* gfortran.dg/goacc/derived-types.f90: New test.
* gfortran.dg/goacc/derived-types-2.f90: New test.
* gfortran.dg/goacc/data-clauses.f95: Adjust for expected errors.
* gfortran.dg/goacc/enter-exit-data.f95: Likewise.
libgomp/
* libgomp.h (struct target_var_desc): Add do_detach flag.
(struct splay_tree_aux): New.
(struct splay_tree_key_s): Replace link_key field with aux pointer.
(gomp_attach_pointer, gomp_detach_pointer): Add prototypes.
* libgomp.map (OACC_2.6): New section. Add acc_attach,
acc_attach_async, acc_detach, acc_detach_async, acc_detach_finalize,
acc_detach_finalize_async.
* oacc-init.c (acc_shutdown_1): Free aux block if present.
* oacc-mem.c (acc_attach_async, acc_attach, goacc_detach_internal,
acc_detach, acc_detach_async, acc_detach_finalize,
acc_detach_finalize_async): New functions.
* oacc-parallel.c (find_group_last): Add SIZES parameter. Support
struct components. Tidy up and add some new checks.
(goacc_enter_data_internal): Update call to find_group_last.
(goacc_exit_data_internal): Support detach operations and
GOMP_MAP_STRUCT.
(GOACC_enter_exit_data): Handle initial GOMP_MAP_STRUCT or
GOMP_MAP_FORCE_PRESENT in finalization detection code. Handle
attach/detach in enter/exit data detection code.
* openacc.h (acc_attach, acc_attach_async, acc_detach,
(acc_detach_async, acc_detach_finalize, acc_detach_finalize_async): Add
prototypes.
* target.c (dump_tgt): Support aux field.
(gomp_map_vars_existing): Initialise do_detach field of tgt_var_desc.
(gomp_attach_pointer, gomp_detach_pointer): New functions.
(gomp_map_vars_internal): Support attach and detach.
(gomp_remove_var_internal): Free aux block and attachment counts if
present.
(gomp_unmap_vars_internal): Support detach.
(gomp_load_image_to_device): Zero-initialise aux field instead of
link_key field.
(gomp_exit_data): Handle link key in aux field. Free aux field when
appropriate.
(omp_target_associate_ptr): Zero-initialize aux field instead of
link_key.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-1.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-2.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-3.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-4.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-5.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-6.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-7.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-8.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-9.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-10.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-11.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-14.c: New test.
* testsuite/libgomp.oacc-c++/deep-copy-12.C: New test.
* testsuite/libgomp.oacc-c++/deep-copy-13.C: New test.
* testsuite/libgomp.oacc-fortran/deep-copy-1.f90: New test.
* testsuite/libgomp.oacc-fortran/deep-copy-2.f90: New test.
* testsuite/libgomp.oacc-fortran/deep-copy-3.f90: New test.
* testsuite/libgomp.oacc-fortran/deep-copy-4.f90: New test.
* testsuite/libgomp.oacc-fortran/deep-copy-5.f90: New test.
* testsuite/libgomp.oacc-fortran/deep-copy-6.f90: New test.
* testsuite/libgomp.oacc-fortran/deep-copy-7.f90: New test.
* testsuite/libgomp.oacc-fortran/deep-copy-8.f90: New test.
* testsuite/libgomp.oacc-fortran/derived-type-1.f90: New test.
* testsuite/libgomp.oacc-fortran/derivedtype-1.f95: New test.
* testsuite/libgomp.oacc-fortran/derivedtype-2.f95: New test.
* testsuite/libgomp.oacc-fortran/multidim-slice.f95: New test.
* testsuite/libgomp.oacc-fortran/update-2.f90: New test.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: attach-detach-fsf-20191106.diff
Type: text/x-patch
Size: 175823 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20191106/cf79c4ab/attachment.bin>
More information about the Gcc-patches
mailing list