Re: [patch] OpenMP/Fortran: Implement omp allocators/allocate for ptr/allocatables

[Thomas Schwinge <thomas@codesourcery.com>]

Hi Tobias!

On 2023-11-08T17:58:10+0100, Tobias Burnus <tobias@codesourcery.com> wrote:
> OpenMP/Fortran: Implement omp allocators/allocate for ptr/allocatables

Nice work!

> This commit adds -fopenmp-allocators which enables support for
> 'omp allocators' and 'omp allocate' that are associated with a Fortran
> allocate-stmt. If such a construct is encountered, an error is shown,
> unless the -fopenmp-allocators flag is present.
>
> With -fopenmp -fopenmp-allocators, those constructs get turned into
> GOMP_alloc allocations, while -fopenmp-allocators (also without -fopenmp)
> ensures deallocation and reallocation (via intrinsic assignments) are
> properly directed to GOMP_free/omp_realloc - while normal Fortran
> allocations are processed by free/realloc.
>
> In order to distinguish a 'malloc'ed from a 'GOMP_alloc'ed memory, the
> version field of the Fortran array discriptor is (mis)used: 0 indicates
> the normal Fortran allocation while 1 denotes GOMP_alloc. For scalars,
> there is record keeping in libgomp: GOMP_add_alloc(ptr) will add the
> pointer address to a splay_tree while GOMP_is_alloc(ptr) will return
> true it was previously added but also removes it from the list.
>
> Besides Fortran FE work, BUILT_IN_GOMP_REALLOC is no part of
> omp-builtins.def and libgomp gains the mentioned two new function.

Minor comments:

> --- a/gcc/fortran/trans-openmp.cc
> +++ b/gcc/fortran/trans-openmp.cc

> +/* Add ptr for tracking as being allocated by GOMP_alloc. */
> +
> +tree
> +gfc_omp_call_add_alloc (tree ptr)
> +{
> +  static tree fn = NULL_TREE;
> +  if (fn == NULL_TREE)
> +    {
> +      fn = build_function_type_list (void_type_node, ptr_type_node, NULL_TREE);
> +      fn = build_fn_decl ("GOMP_add_alloc", fn);
> +/* FIXME: attributes.  */
> +    }
> +  return build_call_expr_loc (input_location, fn, 1, ptr);
> +}
> +
> +/* Generated function returns true when it was tracked via GOMP_add_alloc and
> +   removes it from the tracking.  As called just before GOMP_free or omp_realloc
> +   the pointer is or might become invalid, thus, it is always removed. */
> +
> +tree
> +gfc_omp_call_is_alloc (tree ptr)
> +{
> +  static tree fn = NULL_TREE;
> +  if (fn == NULL_TREE)
> +    {
> +      fn = build_function_type_list (boolean_type_node, ptr_type_node,
> +                                  NULL_TREE);
> +      fn = build_fn_decl ("GOMP_is_alloc", fn);
> +/* FIXME: attributes.  */
> +    }
> +  return build_call_expr_loc (input_location, fn, 1, ptr);
> +}

Why not define 'GOMP_add_alloc', 'GOMP_is_alloc' via
'gcc/omp-builtins.def'?

> --- a/gcc/omp-builtins.def
> +++ b/gcc/omp-builtins.def
> @@ -467,6 +467,9 @@ DEF_GOMP_BUILTIN (BUILT_IN_GOMP_WORKSHARE_TASK_REDUCTION_UNREGISTER,
>  DEF_GOMP_BUILTIN (BUILT_IN_GOMP_ALLOC,
>                 "GOMP_alloc", BT_FN_PTR_SIZE_SIZE_PTRMODE,
>                 ATTR_ALLOC_WARN_UNUSED_RESULT_SIZE_2_NOTHROW_LIST)
> +DEF_GOMP_BUILTIN (BUILT_IN_GOMP_REALLOC,
> +               "omp_realloc", BT_FN_PTR_PTR_SIZE_PTRMODE_PTRMODE,
> +               ATTR_ALLOC_WARN_UNUSED_RESULT_SIZE_2_NOTHROW_LEAF_LIST)
>  DEF_GOMP_BUILTIN (BUILT_IN_GOMP_FREE,
>                 "GOMP_free", BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST)
>  DEF_GOMP_BUILTIN (BUILT_IN_GOMP_WARNING, "GOMP_warning",

Should this either be 'BUILT_IN_OMP_REALLOC' ('OMP' instead of 'GOMP'),
or otherwise a 'GOMP_realloc' be added to 'libgomp/allocator.c', like for
'GOMP_alloc', 'GOMP_free'; 'ialias_call'ing the respective 'omp_[...]'
functions?  (Verbatim 'omp_realloc' also mentioned in the comment before
'gcc/fortran/trans-openmp.cc:gfc_omp_call_is_alloc'.)

> --- a/libgomp/allocator.c
> +++ b/libgomp/allocator.c

> +/* Add pointer as being alloced by GOMP_alloc.  */
> +void
> +GOMP_add_alloc (void *ptr)
> +{
> +  [...]
> +}
> +
> +/* Remove pointer, either called by FREE or by REALLOC,
> +   either of them can change the allocation status.  */
> +bool
> +GOMP_is_alloc (void *ptr)
> +{
> +  [...]
> +}

> --- a/libgomp/libgomp.map
> +++ b/libgomp/libgomp.map

> +GOMP_5.1.2 {
> +  global:
> +     GOMP_add_alloc;
> +     GOMP_is_alloc;
> +     [...]
> +} GOMP_5.1.1;

'GOMP_add_alloc', 'GOMP_is_alloc' should get prototyped in
'libgomp/libgomp_g.h'.


Grüße
 Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955