Re: [PATCH, v2, OpenMP 5.0, libgomp] Structure element mapping for OpenMP 5.0

[Chung-Lin Tang <cltang@codesourcery.com>]

Ping x2.

Hi Jakub, would like this part of OpenMP 5.0 to be considered for GCC 11.

Thanks,
Chung-Lin

On 2020/12/14 6:32 PM, Chung-Lin Tang wrote:
> Ping.
> 
> On 2020/12/4 10:15 PM, Chung-Lin Tang wrote:
>> Hi Jakub,
>> this is a new version of the structure element mapping patch for OpenMP 5.0 requirement
>> changes.
>>
>> This one uses the approach you've outlined in your concept patch [1], basically to
>> use more special REFCOUNT_* values to mark them, and link following structure element
>> splay_tree_keys back to the first key's refcount.
>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557622.html
>>
>> Implementation notes of the attached patch:
>>
>> (1) This patch solves the 5.0 requirements of "not already incremented/decremented
>> because of the effect of a map clause on the construct" by pulling in libgomp/hashtab.h
>> and using htab_t as a pointer set. A "htab_t *refcount_set" is added in map/unmap
>> routines to track the processing status of the uintptr_t* addresses of refcount
>> fields in splay_tree_keys.
>>
>>     * Currently this patch is using the same htab_create/htab_free routines like in task.c.
>>       I toyed with creating a 'htab_alloca' macro (allocating a fixed size htab) to speed
>>       things further, but decided to play it safer for the current patch.
>>
>> (2) Because of the use of pointer-to-refcounts as the basis, and structure element
>> siblings all share a same refcount, uniform increment/decrement without repeating is
>> also naturally achieved.
>>
>> (3) Because of the need to remove whole structure element sibling sequences out of
>> context, it appears we need to mark the first/last of such a sequence. You'll see that
>> the special REFCOUNT_* values have been expanded a bit more than your concept patch
>> (at some point we should think about stop abusing it and add a proper flags word)
>>
>> (4) The new increment/decrement routines combine most of the new refcount_set lookup
>> code with the refcount adjusting. For the decrement routine, "copy" and "removal" are
>> now separate return values, since for structure element sequences, even when signalling
>> "removal" you may still need to finish the "copy" work of following target_var_descs.
>>
>> (5) There are some re-organizing changes to oacc-parallel.c and oacc-mem.c, but most
>> of the code that matters is in target.c.
>>
>> (6) New testcases have been added to reflect the cases discussed on omp-lang list.
>>
>> This patch has been tested for libgomp with no regressions on x86_64-linux with
>> nvptx offloading. Since I submitted the first "v1" patch long ago, is this okay to be
>> considered as committable now after approval?
>>
>> Thanks,
>> Chung-Lin
>>
>> 2020-12-04  Chung-Lin Tang  <cltang@codesourcery.com>
>>
>>      libgomp/
>>      * hashtab.h (htab_clear): New function with initialization code
>>      factored out from...
>>      (htab_create): ...here, adjust to use htab_clear function.
>>
>>      * libgomp.h (REFCOUNT_SPECIAL): New symbol to denote range of
>>      special refcount values, add comments.
>>      (REFCOUNT_INFINITY): Adjust definition to use REFCOUNT_SPECIAL.
>>      (REFCOUNT_LINK): Likewise.
>>      (REFCOUNT_STRUCTELEM): New special refcount range for structure
>>      element siblings.
>>      (REFCOUNT_STRUCTELEM_P): Macro for testing for structure element
>>      sibling maps.
>>      (REFCOUNT_STRUCTELEM_FLAG_FIRST): Flag to indicate first sibling.
>>      (REFCOUNT_STRUCTELEM_FLAG_LAST):  Flag to indicate last sibling.
>>      (REFCOUNT_STRUCTELEM_FIRST_P): Macro to test _FIRST flag.
>>      (REFCOUNT_STRUCTELEM_LAST_P): Macro to test _LAST flag.
>>      (struct splay_tree_key_s): Add structelem_refcount and
>>      structelem_refcount_ptr fields into a union with dynamic_refcount.
>>      Add comments.
>>      (gomp_map_vars): Delete declaration.
>>      (gomp_map_vars_async): Likewise.
>>      (gomp_unmap_vars): Likewise.
>>      (gomp_unmap_vars_async): Likewise.
>>      (goacc_map_vars): New declaration.
>>      (goacc_unmap_vars): Likewise.
>>
>>      * oacc-mem.c (acc_map_data): Adjust to use goacc_map_vars.
>>      (goacc_enter_datum): Likewise.
>>      (goacc_enter_data_internal): Likewise.
>>      * oacc-parallel.c (GOACC_parallel_keyed): Adjust to use goacc_map_vars
>>      and goacc_unmap_vars.
>>      (GOACC_data_start): Adjust to use goacc_map_vars.
>>      (GOACC_data_end): Adjust to use goacc_unmap_vars.
>>
>>      * target.c (hash_entry_type): New typedef.
>>      (htab_alloc): New function hook for hashtab.h.
>>      (htab_free): Likewise.
>>      (htab_hash): Likewise.
>>      (htab_eq): Likewise.
>>      (hashtab.h): Add file include.
>>      (gomp_increment_refcount): New function.
>>      (gomp_decrement_refcount): Likewise.
>>      (gomp_map_vars_existing): Add refcount_set parameter, adjust to use
>>      gomp_increment_refcount.
>>      (gomp_map_fields_existing): Add refcount_set parameter, adjust calls
>>      to gomp_map_vars_existing.
>>
>>      (gomp_map_vars_internal): Add refcount_set parameter, add local openmp_p
>>      variable to guard OpenMP specific paths, adjust calls to
>>      gomp_map_vars_existing, add structure element sibling splay_tree_key
>>      sequence creation code, adjust Fortran map case to avoid increment
>>      under OpenMP.
>>      (gomp_map_vars): Adjust to static, add refcount_set parameter, manage
>>      local refcount_set if caller passed in NULL, adjust call to
>>      gomp_map_vars_internal.
>>      (gomp_map_vars_async): Adjust and rename into...
>>      (goacc_map_vars): ...this new function, adjust call to
>>      gomp_map_vars_internal.
>>
>>      (gomp_remove_splay_tree_key): New function with code factored out from
>>      gomp_remove_var_internal.
>>      (gomp_remove_var_internal): Add code to handle removing multiple
>>      splay_tree_key sequence for structure elements, adjust code to use
>>      gomp_remove_splay_tree_key for splay-tree key removal.
>>      (gomp_unmap_vars_internal): Add refcount_set parameter, adjust to use
>>      gomp_decrement_refcount.
>>      (gomp_unmap_vars): Adjust to static, add refcount_set parameter, manage
>>      local refcount_set if caller passed in NULL, adjust call to
>>      gomp_unmap_vars_internal.
>>      (gomp_unmap_vars_async): Adjust and rename into...
>>      (goacc_unmap_vars): ...this new function, adjust call to
>>      gomp_unmap_vars_internal.
>>      (GOMP_target): Manage refcount_set and adjust calls to gomp_map_vars and
>>      gomp_unmap_vars.
>>      (GOMP_target_ext): Likewise.
>>      (gomp_target_data_fallback): Adjust call to gomp_map_vars.
>>      (GOMP_target_data): Likewise.
>>      (GOMP_target_data_ext): Likewise.
>>      (GOMP_target_end_data): Adjust call to gomp_unmap_vars.
>>      (gomp_exit_data): Add refcount_set parameter, adjust to use
>>      gomp_decrement_refcount, adjust to queue splay-tree keys for removal
>>      after main loop.
>>      (GOMP_target_enter_exit_data): Manage refcount_set and adjust calls to
>>      gomp_map_vars and gomp_exit_data.
>>      (gomp_target_task_fn): Likewise.
>>
>>      * testsuite/libgomp.c-c++-common/refcount-1.c: New testcase.
>>      * testsuite/libgomp.c-c++-common/struct-elem-1.c: New testcase.
>>      * testsuite/libgomp.c-c++-common/struct-elem-2.c: New testcase.
>>      * testsuite/libgomp.c-c++-common/struct-elem-3.c: New testcase.
>>      * testsuite/libgomp.c-c++-common/struct-elem-4.c: New testcase.
>>      * testsuite/libgomp.c-c++-common/struct-elem-5.c: New testcase.