[Patch][OpenMP] use_device_addr/use_device_ptr with Fortran allocatable/pointer arrays (= array descriptor)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[Patch][OpenMP] use_device_addr/use_device_ptr with Fortran allocatable/pointer arrays (= array descriptor)

Tobias Burnus-3
This is a follow-up patch to the use_device_addr work. In particular, it
replaces the patch at https://gcc.gnu.org/ml/fortran/2019-09/msg00088.html

Fortran array descriptors need special handling, which this patch adds.

[The use_device-addr-{3,4} test cases are based on the
use_device_addr-{1,2} ones; I saw that I could have tested there
optional + deallocated in addition, hence, I added it as well (to all
four files). – The use_device-ptr-1.f90 is a slightly cleaned-up version
of previous patch and I have removed the compile-time checks of previous
patch.]

Bootstrapped + regtested on x86_64-gnu-linux w/o "device". And tested
additionally the testcases with nvptx offloading.

Thanks,

Tobias


use_device_addr_desc.diff (100K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patch][OpenMP] use_device_addr/use_device_ptr with Fortran allocatable/pointer arrays (= array descriptor)

Jakub Jelinek
On Mon, Oct 14, 2019 at 03:11:43PM +0200, Tobias Burnus wrote:
> gcc/fortran/
> * f95-lang.c (LANG_HOOKS_OMP_ARRAY_DATA): Set to gfc_omp_array_data.
> * trans-array.c

Description of what has been changed and how is missing.

> --- a/gcc/langhooks.h
> +++ b/gcc/langhooks.h
> @@ -222,6 +222,10 @@ struct lang_hooks_for_decls
>    /* True if this decl may be called via a sibcall.  */
>    bool (*ok_for_sibcall) (const_tree);
>  
> +  /* Return a tree for of the actual data of an array descriptor - or

s/for of/for/ ?

> +     NULL_TREE if original tree is not an array descriptor.  */
> +  tree (*omp_array_data) (tree);
> +

> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -90,6 +90,7 @@ struct omp_context
>    /* Map variables to fields in a structure that allows communication
>       between sending and receiving threads.  */
>    splay_tree field_map;
> +  splay_tree array_data_map;

I was hoping you could get away without introducing yet another splay_tree
for this.  As I said on IRC, we already do have the need to record two
different fields for the same decl and do that through using
the decl address (var) once as the splay_tree_key and in another case
as &DECL_UID (var).  So can't you use the mask & 16 mode to use say
&DECL_NAME (var) and pass the decl to install_var_field rather than x?

> -  if ((mask & 8) != 0)
> +  if ((mask & 16) != 0)
> +    key = (splay_tree_key) var;

That would be here.

> +  else if ((mask & 8) != 0)
>      {
>        key = (splay_tree_key) &DECL_UID (var);
>        gcc_checking_assert (key != (splay_tree_key) var);
> @@ -745,14 +748,17 @@ install_var_field (tree var, bool by_ref, int mask, omp_context *ctx)
>    else if ((mask & 3) == 1 && omp_is_reference (var))
>      type = TREE_TYPE (type);
>  
> -  field = build_decl (DECL_SOURCE_LOCATION (var),
> -      FIELD_DECL, DECL_NAME (var), type);
> +  if ((mask & 16) != 0)
> +    field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE, type);
> +  else
> +    field = build_decl (DECL_SOURCE_LOCATION (var),
> + FIELD_DECL, DECL_NAME (var), type);

And you could use the DECL_NAME as well as DECL_SOURCE_LOCATION.
It is true that for mode & 16 it would need to use the target hook to
actually find out the type of the field, so perhaps we'd need to call
the new target hook multiple times.  As it creates trees that is not the
best idea, though perhaps it could have an extra bool argument where it
would return the type of the data pointer in the descriptor (+ info whether
it is a descriptor through whether it returns non-NULL) with false for the
new arg, something that would be cheap to be called multiple times, once in
scan_sharing_clauses, then again in install_var_field for mode & 16, and
with true in the hopefully only spot where we need the actual expression
(where we gimplify it and store into the corresponding field).

> @@ -1070,7 +1078,7 @@ fixup_child_record_type (omp_context *ctx)
>  static void
>  scan_sharing_clauses (tree clauses, omp_context *ctx)
>  {
> -  tree c, decl;
> +  tree c, decl, x;
>    bool scan_array_reductions = false;
>  
>    for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
> @@ -1240,10 +1248,33 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
>   case OMP_CLAUSE_USE_DEVICE_PTR:
>   case OMP_CLAUSE_USE_DEVICE_ADDR:
>    decl = OMP_CLAUSE_DECL (c);
> -  if ((OMP_CLAUSE_CODE (c) == OMP_CLAUSE_USE_DEVICE_ADDR
> -       && !omp_is_reference (decl)
> -       && !omp_is_allocatable_or_ptr (decl))
> -      || TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
> +          x = NULL;
> +  // Handle array descriptors

Most of omp-low.c uses /* ... */ comments, can you use them here too
(plus dot at the end + two spaces before */)?

> +  if (TREE_CODE (TREE_TYPE (decl)) == RECORD_TYPE ||

Formatting, || needs to be on the following line.

> +      (omp_is_reference (decl)
> +       && TREE_CODE (TREE_TYPE (TREE_TYPE (decl))) == RECORD_TYPE))
> +    x = lang_hooks.decls.omp_array_data (decl);
> +
> +  if (x)
> +    {
> +      gcc_assert (!ctx->array_data_map
> +  || !splay_tree_lookup (ctx->array_data_map,
> +       (splay_tree_key) decl));
> +      if (!ctx->array_data_map)
> + ctx->array_data_map
> + = splay_tree_new (splay_tree_compare_pointers, 0, 0);

Formatting, indented too much (though, see above, I'd hope you can avoid
that).

> +
> +      splay_tree_insert (ctx->array_data_map, (splay_tree_key) decl,
> + (splay_tree_value) x);
> +
> +      install_var_field (x, false, 19, ctx);
> +      DECL_SOURCE_LOCATION (lookup_field (x, ctx))
> + = OMP_CLAUSE_LOCATION (c);

Formatting.

> -    if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_IS_DEVICE_PTR)
> +
> +    // For arrays with descriptor, use the pointer to the actual data

Comment style.

> + tkind = OMP_CLAUSE_CODE (c) != OMP_CLAUSE_IS_DEVICE_PTR
> + ? GOMP_MAP_USE_DEVICE_PTR : GOMP_MAP_FIRSTPRIVATE_INT;

I think emacs users (I'm not one of them) prefer here
                tkind = (OMP_...
                         ? ...);

        Jakub

Reply | Threaded
Open this post in threaded view
|

Re: [Patch][OpenMP] use_device_addr/use_device_ptr with Fortran allocatable/pointer arrays (= array descriptor)

Thomas Schwinge-8
Hi!

On 2019-10-31T18:09:28+0100, Tobias Burnus <[hidden email]> wrote:
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/use_device_addr-3.f90

> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/use_device_addr-4.f90

> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/use_device_ptr-1.f90

As obvious; see attached, committed "Torture testing:
'libgomp.fortran/use_device_addr-3.f90',
'libgomp.fortran/use_device_addr-4.f90',
'libgomp.fortran/use_device_ptr-1.f90'" to trunk in r278044.


Grüße
 Thomas



From d7a5b0d74344a5012fb46a1ccb5b1b94b8110c50 Mon Sep 17 00:00:00 2001
From: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Mon, 11 Nov 2019 08:50:29 +0000
Subject: [PATCH] Torture testing: 'libgomp.fortran/use_device_addr-3.f90',
 'libgomp.fortran/use_device_addr-4.f90',
 'libgomp.fortran/use_device_ptr-1.f90'

        libgomp/
        * testsuite/libgomp.fortran/use_device_addr-3.f90: Specify 'dg-do
        run'.
        * testsuite/libgomp.fortran/use_device_addr-4.f90: Likewise.
        * testsuite/libgomp.fortran/use_device_ptr-1.f90: Likewise.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@278044 138bc75d-0d04-0410-961f-82ee72b054a4
---
 libgomp/ChangeLog                                       | 7 +++++++
 libgomp/testsuite/libgomp.fortran/use_device_addr-3.f90 | 2 ++
 libgomp/testsuite/libgomp.fortran/use_device_addr-4.f90 | 2 ++
 libgomp/testsuite/libgomp.fortran/use_device_ptr-1.f90  | 2 ++
 4 files changed, 13 insertions(+)

diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog
index 067d247da9ac..0e73cadb6cd0 100644
--- a/libgomp/ChangeLog
+++ b/libgomp/ChangeLog
@@ -1,3 +1,10 @@
+2019-11-11  Thomas Schwinge  <[hidden email]>
+
+ * testsuite/libgomp.fortran/use_device_addr-3.f90: Specify 'dg-do
+ run'.
+ * testsuite/libgomp.fortran/use_device_addr-4.f90: Likewise.
+ * testsuite/libgomp.fortran/use_device_ptr-1.f90: Likewise.
+
 2019-11-06  Thomas Schwinge  <[hidden email]>
 
  * testsuite/libgomp.oacc-c-c++-common/par-loop-comb-reduction-1.c:
diff --git a/libgomp/testsuite/libgomp.fortran/use_device_addr-3.f90 b/libgomp/testsuite/libgomp.fortran/use_device_addr-3.f90
index 6d794d74cb3e..5c42bee718ce 100644
--- a/libgomp/testsuite/libgomp.fortran/use_device_addr-3.f90
+++ b/libgomp/testsuite/libgomp.fortran/use_device_addr-3.f90
@@ -1,3 +1,5 @@
+! { dg-do run }
+
 ! Comprehensive run-time test for use_device_addr
 !
 ! Tests array with array descriptor
diff --git a/libgomp/testsuite/libgomp.fortran/use_device_addr-4.f90 b/libgomp/testsuite/libgomp.fortran/use_device_addr-4.f90
index 32dc92c2ff4b..5e66a79da90b 100644
--- a/libgomp/testsuite/libgomp.fortran/use_device_addr-4.f90
+++ b/libgomp/testsuite/libgomp.fortran/use_device_addr-4.f90
@@ -1,3 +1,5 @@
+! { dg-do run }
+
 ! Comprehensive run-time test for use_device_addr
 !
 ! Tests array with array descriptor
diff --git a/libgomp/testsuite/libgomp.fortran/use_device_ptr-1.f90 b/libgomp/testsuite/libgomp.fortran/use_device_ptr-1.f90
index 6428beb357c8..e5390e27a512 100644
--- a/libgomp/testsuite/libgomp.fortran/use_device_ptr-1.f90
+++ b/libgomp/testsuite/libgomp.fortran/use_device_ptr-1.f90
@@ -1,3 +1,5 @@
+! { dg-do run }
+
 module target_procs
   use iso_c_binding
   implicit none (type, external)
--
2.17.1


signature.asc (671 bytes) Download Attachment