date:20220202

Re: [PATCH] C, C++, Fortran, OpenMP: Add 'has_device_addr' clause to 'target' construct

2022-02-02 Thread Marcel Vollweiler


Hi Jakub,


+case OMP_CLAUSE_HAS_DEVICE_ADDR:
+  t = OMP_CLAUSE_DECL (c);
+  if (TREE_CODE (t) == TREE_LIST)
+{
+  if (handle_omp_array_sections (c, ort))
+remove = true;
+  else
+{
+  t = OMP_CLAUSE_DECL (c);
+  while (TREE_CODE (t) == ARRAY_REF)
+t = TREE_OPERAND (t, 0);
+}
+}
+  if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_HAS_DEVICE_ADDR)
+bitmap_set_bit (&is_on_device_head, DECL_UID (t));


Why the OMP_CLAUSE_CODE (c) == OMP_CLAUSE_HAS_DEVICE_ADDR check?
There is no goto into this block nor fallthru into it, and
handle_omp_array_sections better shouldn't change OMP_CLAUSE_CODE.


Good point. Removed.




   goto check_dup_generic;

+case OMP_CLAUSE_HAS_DEVICE_ADDR:
+  t = OMP_CLAUSE_DECL (c);
+  if (TREE_CODE (t) == TREE_LIST)
+if (handle_omp_array_sections (c, ort))
+  remove = true;
+else
+  {
+t = OMP_CLAUSE_DECL (c);
+while (TREE_CODE (t) == ARRAY_REF)
+  t = TREE_OPERAND (t, 0);
+  }
+  if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_HAS_DEVICE_ADDR)
+bitmap_set_bit (&is_on_device_head, DECL_UID (t));


Likewise.


Removed.




+  if (VAR_P (t) || TREE_CODE (t) == PARM_DECL)
+cxx_mark_addressable (t);
+  goto check_dup_generic_t;
+
 case OMP_CLAUSE_USE_DEVICE_ADDR:
   field_ok = true;
   t = OMP_CLAUSE_DECL (c);



--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -1391,7 +1391,8 @@ enum
OMP_LIST_USE_DEVICE_PTR,
OMP_LIST_USE_DEVICE_ADDR,
OMP_LIST_NONTEMPORAL,
-  OMP_LIST_NUM
+  OMP_LIST_HAS_DEVICE_ADDR,
+  OMP_LIST_NUM  /* must be the last  */


Capital M and . at the end.


Changed.




@@ -2077,6 +2078,12 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const 
omp_mask mask,
 }
   break;
 case 'h':
+  if ((mask & OMP_CLAUSE_HAS_DEVICE_ADDR)
+  && gfc_match_omp_variable_list
+   ("has_device_addr (",
+&c->lists[OMP_LIST_HAS_DEVICE_ADDR], false, NULL, NULL,
+ true) == MATCH_YES)


Formatting, true should be IMO below &c->lists.


Corrected the formatting.




+continue;
   if ((mask & OMP_CLAUSE_HINT)
   && (m = gfc_match_dupl_check (!c->hint, "hint", true, &c->hint))
  != MATCH_NO)
@@ -2850,7 +2857,8 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const 
omp_mask mask,
   if ((mask & OMP_CLAUSE_USE_DEVICE_ADDR)
   && gfc_match_omp_variable_list
("use_device_addr (",
-&c->lists[OMP_LIST_USE_DEVICE_ADDR], false) == MATCH_YES)
+&c->lists[OMP_LIST_USE_DEVICE_ADDR], false, NULL, NULL,
+ true) == MATCH_YES)


Likewise.


Corrected.




--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -1910,7 +1910,17 @@ gfc_trans_omp_variable_list (enum omp_clause_code code,
 tree t = gfc_trans_omp_variable (namelist->sym, declare_simd);
 if (t != error_mark_node)
   {
-tree node = build_omp_clause (input_location, code);
+tree node;
+/* For HAS_DEVICE_ADDR of an array descriptor, firstprivatize the
+   descriptor such that the bounds are available; its data component
+   is unmodified; it is handled as device address inside target. */
+if (code == OMP_CLAUSE_HAS_DEVICE_ADDR
+&& (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (t))
+|| (POINTER_TYPE_P (TREE_TYPE (t))
+&& GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (TREE_TYPE (t))
+  node = build_omp_clause (input_location, OMP_CLAUSE_FIRSTPRIVATE);


Not sure about the above,


This is needed for allocatable arrays and array pointers to ensure that
not only the (array) data is (already) present on the device but also
the array descriptor. Otherwise the test cases
target-has-device-addr-2.f90, target-has-device-addr-3.f90 (because of
variable "c") and target-has-device-addr-4.f90 (also because of variable
"c") won't work.




--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -10024,6 +10024,15 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq 
*pre_p,
   flags = GOVD_EXPLICIT;
   goto do_add;

+case OMP_CLAUSE_HAS_DEVICE_ADDR:
+  decl = OMP_CLAUSE_DECL (c);
+  if (TREE_CODE (decl) == ARRAY_REF)
+{
+  flags = GOVD_FIRSTPRIVATE | GOVD_EXPLICIT;
+  while (TREE_CODE (decl) == ARRAY_REF)
+decl = TREE_OPERAND (decl, 0);
+  goto do_add_decl;


but this looks weird.
If decl after stripping the ARRAY_REFs is a var with pointer type, sure,
firstprivatizing it is the way to go.
But it can be also a variable with ARRAY_TYPE, can't it?  Something like:
   int a[64];
   #pragma omp target data map(a) use_device_addr(a)
   {
 #pragma omp target has_device_addr(a[3:16])
 a[3] = 1;
   }
and

Re: [PATCH] C, C++, Fortran, OpenMP: Add 'has_device_addr' clause to 'target' construct

2022-02-02 Thread Jakub Jelinek via Fortran

On Wed, Feb 02, 2022 at 09:19:03AM +0100, Marcel Vollweiler wrote:
> gcc/c-family/ChangeLog:
> 
>   * c-omp.cc (c_omp_split_clauses): Added OMP_CLAUSE_HAS_DEVICE_ADDR case.
>   * c-pragma.h (enum pragma_kind): Added 5.1 in comment.
>   (enum pragma_omp_clause): Added PRAGMA_OMP_CLAUSE_HAS_DEVICE_ADDR.
> 
> gcc/c/ChangeLog:
> 
>   * c-parser.cc (c_parser_omp_clause_name): Parse 'has_device_addr'
>   clause.
>   (c_parser_omp_variable_list): Handle array sections.
>   (c_parser_omp_clause_has_device_addr): Added.
>   (c_parser_omp_all_clauses): Added PRAGMA_OMP_CLAUSE_HAS_DEVICE_ADDR
>   case.
>   (c_parser_omp_target_exit_data): Added HAS_DEVICE_ADDR to 
>   OMP_CLAUSE_MASK.
>   * c-typeck.cc (handle_omp_array_sections): Handle clause restrictions.
>   (c_finish_omp_clauses): Handle array sections.
> 
> gcc/cp/ChangeLog:
> 
>   * parser.cc (cp_parser_omp_clause_name): Parse 'has_device_addr' clause.
>   (cp_parser_omp_var_list_no_open): Handle array sections.
>   (cp_parser_omp_all_clauses): Added PRAGMA_OMP_CLAUSE_HAS_DEVICE_ADDR
>   case.
>   (cp_parser_omp_target_update): Added HAS_DEVICE_ADDR to OMP_CLAUSE_MASK.
>   * pt.c (tsubst_omp_clauses): Added cases for OMP_CLAUSE_HAS_DEVICE_ADDR.
>   * semantics.cc (handle_omp_array_sections): Handle clause restrictions.
>   (finish_omp_clauses): Handle array sections.
> 
> gcc/fortran/ChangeLog:
> 
>   * dump-parse-tree.cc (show_omp_clauses): Added OMP_LIST_HAS_DEVICE_ADDR
>   case.
>   * gfortran.h: Added OMP_LIST_HAS_DEVICE_ADDR.
>   * openmp.cc (enum omp_mask2): Added OMP_CLAUSE_HAS_DEVICE_ADDR.
>   (gfc_match_omp_clauses): Parse HAS_DEVICE_ADDR clause.
>   (resolve_omp_clauses): Same.
>   * trans-openmp.cc (gfc_trans_omp_variable_list): Added 
>   OMP_LIST_HAS_DEVICE_ADDR case.
>   (gfc_trans_omp_clauses): Firstprivatize of array descriptors.
> 
> gcc/ChangeLog:
> 
>   * gimplify.cc (gimplify_scan_omp_clauses): Added cases for
>   OMP_CLAUSE_HAS_DEVICE_ADDR
>   and handle array sections.
>   (gimplify_adjust_omp_clauses): Added OMP_CLAUSE_HAS_DEVICE_ADDR case.
>   * omp-low.cc (scan_sharing_clauses): Handle OMP_CLAUSE_HAS_DEVICE_ADDR.
>   (lower_omp_target): Same.
>   * tree-core.h (enum omp_clause_code): Same.
>   * tree-nested.cc (convert_nonlocal_omp_clauses): Same.
>   (convert_local_omp_clauses): Same.
>   * tree-pretty-print.cc (dump_omp_clause): Same.
>   * tree.cc: Same.
> 
> libgomp/ChangeLog:
> 
>   * libgomp.texi: Updated entry for HAS_DEVICE_ADDR.
>   * target.c (copy_firstprivate_data): Copy only if host address is not
>   NULL.
>   * testsuite/libgomp.c++/target-has-device-addr-2.C: New test.
>   * testsuite/libgomp.c++/target-has-device-addr-4.C: New test.
>   * testsuite/libgomp.c++/target-has-device-addr-5.C: New test.
>   * testsuite/libgomp.c++/target-has-device-addr-6.C: New test.
>   * testsuite/libgomp.c-c++-common/target-has-device-addr-1.c: New test.
>   * testsuite/libgomp.c/target-has-device-addr-3.c: New test.
>   * testsuite/libgomp.fortran/target-has-device-addr-1.f90: New test.
>   * testsuite/libgomp.fortran/target-has-device-addr-2.f90: New test.
>   * testsuite/libgomp.fortran/target-has-device-addr-3.f90: New test.
>   * testsuite/libgomp.fortran/target-has-device-addr-4.f90: New test.
> 
> gcc/testsuite/ChangeLog:
> 
>   * c-c++-common/gomp/clauses-1.c: Added has_device_addr to test cases.
>   * g++.dg/gomp/attrs-1.C: Added has_device_addr to test cases.
>   * g++.dg/gomp/attrs-2.C: Added has_device_addr to test cases.
>   * c-c++-common/gomp/target-has-device-addr-1.c: New test.
>   * c-c++-common/gomp/target-has-device-addr-2.c: New test.
>   * c-c++-common/gomp/target-is-device-ptr-1.c: New test.
>   * c-c++-common/gomp/target-is-device-ptr-2.c: New test.
>   * gfortran.dg/gomp/is_device_ptr-3.f90: New test.
>   * gfortran.dg/gomp/target-has-device-addr-1.f90: New test.
>   * gfortran.dg/gomp/target-has-device-addr-2.f90: New test.

Ok, thanks.

Jakub

Re: [PATCH] fortran: Unshare associate var charlen [PR104228]

2022-02-02 Thread Harald Anlauf via Fortran


Hi Mikael,

Am 29.01.22 um 15:24 schrieb Mikael Morin:

Hello,

the attached patch is a fix for PR104228.
Even if simple, I wouldn’t call it obvious, as it’s involving character
length and associate, so I don’t mind some extra review eyes.


I am probably not experienced enough to review this.  Paul?

Nevertheless, I looked at the commits that introduced the code
you touched.  It appears that these did not cover, maybe even
avoid the current case where the associate target is not a dummy.
Your changes seem to make sense and fix the issue.


Tested on x86_64-pc-linux-gnu. Ok for master/11/10/9?


OK, and thanks for the patch!

Harald

Re: [PATCH] C, C++, Fortran, OpenMP: Add 'has_device_addr' clause to 'target' construct

Re: [PATCH] C, C++, Fortran, OpenMP: Add 'has_device_addr' clause to 'target' construct

Re: [PATCH] fortran: Unshare associate var charlen [PR104228]

3 matches

Site Navigation

Mail list logo

Footer information