public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch] OpenMP/Fortran: Add support for enter clause on declare target (was: [committed] openmp: Add support for enter clause on declare target)
       [not found] <YpCt2rJGuydcFlA4@tucnak>
@ 2022-05-27 14:52 ` Tobias Burnus
  2022-05-27 15:20   ` Jakub Jelinek
  0 siblings, 1 reply; 3+ messages in thread
From: Tobias Burnus @ 2022-05-27 14:52 UTC (permalink / raw)
  To: Jakub Jelinek, gcc-patches; +Cc: fortran

[-- Attachment #1: Type: text/plain, Size: 976 bytes --]

This patch adds the Fortran support to the just-committed C/C++ support for the 'enter' clause.

The 'TO'/'ENTER' usage is first stored in a linked list – and
then as attribute to the symbol. I am not sure how to handle it best.
I did went for adding an ENTER_LIST but kept only using the single attribute.

Result: In one diagnostic, I use 'TO or ENTER clause'  in the other,
I can properly use 'TO' or 'ENTER' clause.

This could be kept as is, but to save memory it would be possible
to drop the ENTER_LIST – or, to improve diagnostic, we could try harder
(e.g. by re-walking the list or by another gfc_attribute). Preferences?
If not, I would go for the attached middle way.

Tobias
-----------------
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

[-- Attachment #2: declare-target-enter-f90.diff --]
[-- Type: text/x-patch, Size: 15449 bytes --]

OpenMP/Fortran: Add support for enter clause on declare target

Fortran version to C/C++ commit r13-797-g0ccba4ed8571c18c7015413441e971

gcc/fortran/ChangeLog:

	* dump-parse-tree.cc (show_omp_clauses): Handle OMP_LIST_ENTER.
	* gfortran.h: Add OMP_LIST_ENTER.
	* openmp.cc (enum omp_mask2, OMP_DECLARE_TARGET_CLAUSES): Add
	OMP_CLAUSE_ENTER.
	(gfc_match_omp_clauses, gfc_match_omp_declare_target,
	resolve_omp_clauses): Handle 'enter' clause.

libgomp/ChangeLog:

	* libgomp.texi (OpenMP 5.2): Mark 'enter' clause as supported.
	* testsuite/libgomp.fortran/declare-target-1.f90: Extend to test
	explicit 'to' and 'enter' clause.
	* testsuite/libgomp.fortran/declare-target-2.f90: Update accordingly.

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/declare-target-2.f90: Add 'enter' clause test.
	* gfortran.dg/gomp/declare-target-4.f90: Likewise.

 gcc/fortran/dump-parse-tree.cc                     |  1 +
 gcc/fortran/gfortran.h                             |  1 +
 gcc/fortran/openmp.cc                              | 59 +++++++++++++++-------
 .../gfortran.dg/gomp/declare-target-2.f90          | 18 +++++--
 .../gfortran.dg/gomp/declare-target-4.f90          |  9 +++-
 libgomp/libgomp.texi                               |  2 +-
 .../testsuite/libgomp.fortran/declare-target-1.f90 |  4 +-
 .../testsuite/libgomp.fortran/declare-target-2.f90 | 10 +++-
 8 files changed, 76 insertions(+), 28 deletions(-)

diff --git a/gcc/fortran/dump-parse-tree.cc b/gcc/fortran/dump-parse-tree.cc
index 4e8986bd599..e3affb8bc48 100644
--- a/gcc/fortran/dump-parse-tree.cc
+++ b/gcc/fortran/dump-parse-tree.cc
@@ -1679,6 +1679,7 @@ show_omp_clauses (gfc_omp_clauses *omp_clauses)
 	  case OMP_LIST_IN_REDUCTION: type = "IN_REDUCTION"; break;
 	  case OMP_LIST_TASK_REDUCTION: type = "TASK_REDUCTION"; break;
 	  case OMP_LIST_DEVICE_RESIDENT: type = "DEVICE_RESIDENT"; break;
+	  case OMP_LIST_ENTER: type = "ENTER"; break;
 	  case OMP_LIST_LINK: type = "LINK"; break;
 	  case OMP_LIST_USE_DEVICE: type = "USE_DEVICE"; break;
 	  case OMP_LIST_CACHE: type = "CACHE"; break;
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 5d970bc1df0..0bac8657790 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -1395,6 +1395,7 @@ enum
   OMP_LIST_NONTEMPORAL,
   OMP_LIST_ALLOCATE,
   OMP_LIST_HAS_DEVICE_ADDR,
+  OMP_LIST_ENTER,
   OMP_LIST_NUM /* Must be the last.  */
 };
 
diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index efa62b64f2b..66c30ebea51 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -986,6 +986,7 @@ enum omp_mask2
   OMP_CLAUSE_ATTACH,
   OMP_CLAUSE_NOHOST,
   OMP_CLAUSE_HAS_DEVICE_ADDR,  /* OpenMP 5.1  */
+  OMP_CLAUSE_ENTER, /* OpenMP 5.2 */
   /* This must come last.  */
   OMP_MASK2_LAST
 };
@@ -2101,6 +2102,16 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const omp_mask mask,
 		continue;
 	    }
 	  break;
+	case 'e':
+	  if ((mask & OMP_CLAUSE_ENTER))
+	    {
+	      m = gfc_match_omp_to_link ("enter (", &c->lists[OMP_LIST_ENTER]);
+	      if (m == MATCH_ERROR)
+		goto error;
+	      if (m == MATCH_YES)
+		continue;
+	    }
+	  break;
 	case 'f':
 	  if ((mask & OMP_CLAUSE_FAIL)
 	      && (m = gfc_match_dupl_check (c->fail == OMP_MEMORDER_UNSET,
@@ -2921,8 +2932,12 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const omp_mask mask,
 	    continue;
 	  if ((mask & OMP_CLAUSE_TO) && (mask & OMP_CLAUSE_LINK))
 	    {
-	      if (gfc_match_omp_to_link ("to (", &c->lists[OMP_LIST_TO])
-		  == MATCH_YES)
+	      /* Declare target: 'to' is an alias for 'enter';
+		 'to' is deprecated since 5.2.  */
+	      m = gfc_match_omp_to_link ("to (", &c->lists[OMP_LIST_TO]);
+	      if (m == MATCH_ERROR)
+		goto error;
+	      if (m == MATCH_YES)
 		continue;
 	    }
 	  else if ((mask & OMP_CLAUSE_TO)
@@ -3724,7 +3739,8 @@ cleanup:
 #define OMP_ORDERED_CLAUSES \
   (omp_mask (OMP_CLAUSE_THREADS) | OMP_CLAUSE_SIMD)
 #define OMP_DECLARE_TARGET_CLAUSES \
-  (omp_mask (OMP_CLAUSE_TO) | OMP_CLAUSE_LINK | OMP_CLAUSE_DEVICE_TYPE)
+  (omp_mask (OMP_CLAUSE_ENTER) | OMP_CLAUSE_LINK | OMP_CLAUSE_DEVICE_TYPE \
+   | OMP_CLAUSE_TO)
 #define OMP_ATOMIC_CLAUSES \
   (omp_mask (OMP_CLAUSE_ATOMIC) | OMP_CLAUSE_CAPTURE | OMP_CLAUSE_HINT	\
    | OMP_CLAUSE_MEMORDER | OMP_CLAUSE_COMPARE | OMP_CLAUSE_FAIL 	\
@@ -4530,7 +4546,7 @@ gfc_match_omp_declare_target (void)
     {
       c = gfc_get_omp_clauses ();
       gfc_current_locus = old_loc;
-      m = gfc_match_omp_to_link (" (", &c->lists[OMP_LIST_TO]);
+      m = gfc_match_omp_to_link (" (", &c->lists[OMP_LIST_ENTER]);
       if (m != MATCH_YES)
 	goto syntax;
       if (gfc_match_omp_eos () != MATCH_YES)
@@ -4544,16 +4560,20 @@ gfc_match_omp_declare_target (void)
 
   gfc_buffer_error (false);
 
-  for (list = OMP_LIST_TO; list != OMP_LIST_NUM;
-       list = (list == OMP_LIST_TO ? OMP_LIST_LINK : OMP_LIST_NUM))
+  for (list = OMP_LIST_ENTER; list != OMP_LIST_NUM;
+       list = (list == OMP_LIST_ENTER
+	       ? OMP_LIST_TO
+	       : (list == OMP_LIST_TO ? OMP_LIST_LINK : OMP_LIST_NUM)))
     for (n = c->lists[list]; n; n = n->next)
       if (n->sym)
 	n->sym->mark = 0;
       else if (n->u.common->head)
 	n->u.common->head->mark = 0;
 
-  for (list = OMP_LIST_TO; list != OMP_LIST_NUM;
-       list = (list == OMP_LIST_TO ? OMP_LIST_LINK : OMP_LIST_NUM))
+  for (list = OMP_LIST_ENTER; list != OMP_LIST_NUM;
+       list = (list == OMP_LIST_ENTER
+	       ? OMP_LIST_TO
+	       : (list == OMP_LIST_TO ? OMP_LIST_LINK : OMP_LIST_NUM)))
     for (n = c->lists[list]; n; n = n->next)
       if (n->sym)
 	{
@@ -4564,14 +4584,14 @@ gfc_match_omp_declare_target (void)
 		   && n->sym->attr.omp_declare_target_link
 		   && list != OMP_LIST_LINK)
 	    gfc_error_now ("OMP DECLARE TARGET variable at %L previously "
-			   "mentioned in LINK clause and later in TO clause",
-			   &n->where);
+			   "mentioned in LINK clause and later in %s clause",
+			   &n->where, list == OMP_LIST_TO ? "TO" : "ENTER");
 	  else if (n->sym->attr.omp_declare_target
 		   && !n->sym->attr.omp_declare_target_link
 		   && list == OMP_LIST_LINK)
 	    gfc_error_now ("OMP DECLARE TARGET variable at %L previously "
-			   "mentioned in TO clause and later in LINK clause",
-			   &n->where);
+			   "mentioned in TO or ENTER clause and later in "
+			   "LINK clause", &n->where);
 	  else if (n->sym->mark)
 	    gfc_error_now ("Variable at %L mentioned multiple times in "
 			   "clauses of the same OMP DECLARE TARGET directive",
@@ -4598,14 +4618,14 @@ gfc_match_omp_declare_target (void)
 	       && n->u.common->omp_declare_target_link
 	       && list != OMP_LIST_LINK)
 	gfc_error_now ("OMP DECLARE TARGET COMMON at %L previously "
-		       "mentioned in LINK clause and later in TO clause",
-		       &n->where);
+		       "mentioned in LINK clause and later in %s clause",
+		       &n->where, list == OMP_LIST_TO ? "TO" : "ENTER");
       else if (n->u.common->omp_declare_target
 	       && !n->u.common->omp_declare_target_link
 	       && list == OMP_LIST_LINK)
 	gfc_error_now ("OMP DECLARE TARGET COMMON at %L previously "
-		       "mentioned in TO clause and later in LINK clause",
-		       &n->where);
+		       "mentioned in TO or ENTER clause and later in "
+		       "LINK clause", &n->where);
       else if (n->u.common->head && n->u.common->head->mark)
 	gfc_error_now ("COMMON at %L mentioned multiple times in "
 		       "clauses of the same OMP DECLARE TARGET directive",
@@ -4639,7 +4659,10 @@ gfc_match_omp_declare_target (void)
 	      s->attr.omp_device_type = c->device_type;
 	    }
 	}
-  if (c->device_type && !c->lists[OMP_LIST_TO] && !c->lists[OMP_LIST_LINK])
+  if (c->device_type
+      && !c->lists[OMP_LIST_ENTER]
+      && !c->lists[OMP_LIST_TO]
+      && !c->lists[OMP_LIST_LINK])
     gfc_warning_now (0, "OMP DECLARE TARGET directive at %L with only "
 			"DEVICE_TYPE clause is ignored", &old_loc);
 
@@ -6331,7 +6354,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
 	"IN_REDUCTION", "TASK_REDUCTION",
 	"DEVICE_RESIDENT", "LINK", "USE_DEVICE",
 	"CACHE", "IS_DEVICE_PTR", "USE_DEVICE_PTR", "USE_DEVICE_ADDR",
-	"NONTEMPORAL", "ALLOCATE", "HAS_DEVICE_ADDR" };
+	"NONTEMPORAL", "ALLOCATE", "HAS_DEVICE_ADDR", "ENTER" };
   STATIC_ASSERT (ARRAY_SIZE (clause_names) == OMP_LIST_NUM);
 
   if (omp_clauses == NULL)
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-target-2.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-target-2.f90
index 2217eab07e5..7ac25c84483 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-target-2.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-target-2.f90
@@ -1,9 +1,9 @@
 ! { dg-do compile }
 
 module declare_target_2
-  !$omp declare target to (a) link (a)	! { dg-error "TO clause and later in LINK" }
+  !$omp declare target to (a) link (a)	! { dg-error "TO or ENTER clause and later in LINK" }
   !$omp declare target (b)
-  !$omp declare target link (b)		! { dg-error "TO clause and later in LINK" }
+  !$omp declare target link (b)		! { dg-error "TO or ENTER clause and later in LINK" }
   !$omp declare target link (f)
   !$omp declare target to (f)		! { dg-error "LINK clause and later in TO" }
   !$omp declare target(c, c)		! { dg-error "mentioned multiple times in clauses of the same" }
@@ -39,9 +39,9 @@ subroutine foo				! { dg-error "attribute conflicts" }
   !$omp declare target to (/c2/)
   !$omp declare target (/c2/)
   !$omp declare target to(/c2/)
-  !$omp declare target link(/c2/)	! { dg-error "TO clause and later in LINK" }
+  !$omp declare target link(/c2/)	! { dg-error "TO or ENTER clause and later in LINK" }
   !$omp declare target link(/c3/)
-  !$omp declare target (/c3/)		! { dg-error "LINK clause and later in TO" }
+  !$omp declare target (/c3/)		! { dg-error "LINK clause and later in ENTER" }
   !$omp declare target (/c4/, /c4/)	! { dg-error "mentioned multiple times in clauses of the same" }
   !$omp declare target to (/c4/) to(/c4/) ! { dg-error "mentioned multiple times in clauses of the same" }
   !$omp declare target link (/c5/)
@@ -49,3 +49,13 @@ subroutine foo				! { dg-error "attribute conflicts" }
   !$omp declare target link(/c5/)link(/c5/) ! { dg-error "mentioned multiple times in clauses of the same" }
   !$omp declare target link(/c5/,/c5/)	! { dg-error "mentioned multiple times in clauses of the same" }
 end subroutine
+
+module declare_target_3
+  !$omp declare target enter (a) link (a)	! { dg-error "TO or ENTER clause and later in LINK" }
+  !$omp declare target link(b) enter(b)	! { dg-error "TO or ENTER clause and later in LINK" }
+  !$omp declare target to (c) enter (c)	! { dg-error "mentioned multiple times in clauses of the same" }
+  !$omp declare target enter (d) to (d)	! { dg-error "mentioned multiple times in clauses of the same" }
+  !$omp declare target enter (e) enter (e)	! { dg-error "mentioned multiple times in clauses of the same" }
+  integer, save :: a, b, c, d, e
+end
+
diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-target-4.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-target-4.f90
index 8947b887534..4f5de4bd8c7 100644
--- a/gcc/testsuite/gfortran.dg/gomp/declare-target-4.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/declare-target-4.f90
@@ -21,6 +21,10 @@ subroutine f5
   !$omp declare target device_type (nohost) to (f5)
 end subroutine
 
+subroutine f6
+  !$omp declare target enter (f6) device_type (any)
+end subroutine
+
 module mymod
   ! device_type is ignored for variables in OpenMP 5.0
   ! but TR8 and later apply those rules to variables as well
@@ -69,13 +73,14 @@ module m2
   public :: m, n, o, p, q, r, s, t, u, v, w, x
 end module m2
 
-! { dg-final { scan-tree-dump-times "omp declare target" 7 "original" } }
-! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare target \\(device_type\\(" 7 "original" } }
+! { dg-final { scan-tree-dump-times "omp declare target" 8 "original" } }
+! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare target \\(device_type\\(" 8 "original" } }
 ! { dg-final { scan-tree-dump-not "__attribute__\\(\\(omp declare target \[^\n\r\]*\[\n\r\]void f1" "original" } }
 ! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare target \\(device_type\\(any\\)\\)\\)\\)\[\n\r]__attribute__\[^\n\r]+\[\n\r]void f2" 1 "original" } }
 ! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare target \\(device_type\\(any\\)\\)\\)\\)\[\n\r]__attribute__\[^\n\r]+\[\n\r\]void f3" 1 "original" } }
 ! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare target \\(device_type\\(host\\)\\)\\)\\)\[\n\r]__attribute__\[^\n\r]+\[\n\r\]void f4" 1 "original" } }
 ! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare target \\(device_type\\(nohost\\)\\)\\)\\)\[\n\r]__attribute__\[^\n\r]+\[\n\r\]void f5" 1 "original" } }
+! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare target \\(device_type\\(any\\)\\)\\)\\)\[\n\r]__attribute__\[^\n\r]+\[\n\r]void f6" 1 "original" } }
 ! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare target \\(device_type\\(any\\)\\)\\)\\)\[\n\r]__attribute__\[^\n\r]+\[\n\r\]void s1" 1 "original" } }
 ! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare target \\(device_type\\(nohost\\)\\)\\)\\)\[\n\r]__attribute__\[^\n\r]+\[\n\r\]void s2" 1 "original" } }
 ! { dg-final { scan-tree-dump-times "__attribute__\\(\\(omp declare target \\(device_type\\(host\\)\\)\\)\\)\[\n\r]__attribute__\[^\n\r]+\[\n\r\]void s3" 1 "original" } }
diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi
index 56aab4494ad..ff02ccd4969 100644
--- a/libgomp/libgomp.texi
+++ b/libgomp/libgomp.texi
@@ -368,7 +368,7 @@ The OpenMP 4.5 specification is fully supported.
 @item If a matching mapped list item is not found in the data environment, the
       pointer retains its original value @tab N @tab
 @item New @code{enter} clause as alias for @code{to} on declare target directive
-      @tab N @tab
+      @tab Y @tab
 @item Deprecation of @code{to} clause on declare target directive @tab N @tab
 @item Extended list of directives permitted in Fortran pure procedures
       @tab N @tab
diff --git a/libgomp/testsuite/libgomp.fortran/declare-target-1.f90 b/libgomp/testsuite/libgomp.fortran/declare-target-1.f90
index fd9c26fde95..d9abf685a54 100644
--- a/libgomp/testsuite/libgomp.fortran/declare-target-1.f90
+++ b/libgomp/testsuite/libgomp.fortran/declare-target-1.f90
@@ -2,8 +2,10 @@
 ! { dg-additional-sources declare-target-2.f90 }
 
 module declare_target_1_mod
-  integer :: var_x
+  integer :: var_x, var_y, var_z
   !$omp declare target(var_x)
+  !$omp declare target to(var_y)
+  !$omp declare target enter(var_z)
 end module declare_target_1_mod
 
   interface
diff --git a/libgomp/testsuite/libgomp.fortran/declare-target-2.f90 b/libgomp/testsuite/libgomp.fortran/declare-target-2.f90
index 85fc68c6ede..2210fc5a52f 100644
--- a/libgomp/testsuite/libgomp.fortran/declare-target-2.f90
+++ b/libgomp/testsuite/libgomp.fortran/declare-target-2.f90
@@ -7,12 +7,18 @@ subroutine foo
   use declare_target_1_mod
 
   var_x = 10
-  !$omp target update to(var_x)
+  var_y = 20
+  var_z = 30
+  !$omp target update to(var_x, var_y, var_z)
 
   !$omp target
     var_x = var_x * 2;
+    var_y = var_y * 3;
+    var_z = var_z * 4;
   !$omp end target
 
-  !$omp target update from(var_x)
+  !$omp target update from(var_x, var_y, var_z)
   if (var_x /= 20) stop 1
+  if (var_y /= 20*3) stop 2
+  if (var_z /= 30*4) stop 3
 end subroutine foo

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Patch] OpenMP/Fortran: Add support for enter clause on declare target (was: [committed] openmp: Add support for enter clause on declare target)
  2022-05-27 14:52 ` [Patch] OpenMP/Fortran: Add support for enter clause on declare target (was: [committed] openmp: Add support for enter clause on declare target) Tobias Burnus
@ 2022-05-27 15:20   ` Jakub Jelinek
  2022-05-27 15:44     ` Jakub Jelinek
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2022-05-27 15:20 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: gcc-patches, fortran

On Fri, May 27, 2022 at 04:52:17PM +0200, Tobias Burnus wrote:
> This patch adds the Fortran support to the just-committed C/C++ support for the 'enter' clause.
> 
> The 'TO'/'ENTER' usage is first stored in a linked list – and
> then as attribute to the symbol. I am not sure how to handle it best.
> I did went for adding an ENTER_LIST but kept only using the single attribute.
> 
> Result: In one diagnostic, I use 'TO or ENTER clause'  in the other,
> I can properly use 'TO' or 'ENTER' clause.
> 
> This could be kept as is, but to save memory it would be possible
> to drop the ENTER_LIST – or, to improve diagnostic, we could try harder
> (e.g. by re-walking the list or by another gfc_attribute). Preferences?
> If not, I would go for the attached middle way.
> 
> Tobias
> -----------------
> 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

> OpenMP/Fortran: Add support for enter clause on declare target
> 
> Fortran version to C/C++ commit r13-797-g0ccba4ed8571c18c7015413441e971
> 
> gcc/fortran/ChangeLog:
> 
> 	* dump-parse-tree.cc (show_omp_clauses): Handle OMP_LIST_ENTER.
> 	* gfortran.h: Add OMP_LIST_ENTER.
> 	* openmp.cc (enum omp_mask2, OMP_DECLARE_TARGET_CLAUSES): Add
> 	OMP_CLAUSE_ENTER.
> 	(gfc_match_omp_clauses, gfc_match_omp_declare_target,
> 	resolve_omp_clauses): Handle 'enter' clause.
> 
> libgomp/ChangeLog:
> 
> 	* libgomp.texi (OpenMP 5.2): Mark 'enter' clause as supported.
> 	* testsuite/libgomp.fortran/declare-target-1.f90: Extend to test
> 	explicit 'to' and 'enter' clause.
> 	* testsuite/libgomp.fortran/declare-target-2.f90: Update accordingly.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gfortran.dg/gomp/declare-target-2.f90: Add 'enter' clause test.
> 	* gfortran.dg/gomp/declare-target-4.f90: Likewise.

Mostly good, but see below.

> -  for (list = OMP_LIST_TO; list != OMP_LIST_NUM;
> -       list = (list == OMP_LIST_TO ? OMP_LIST_LINK : OMP_LIST_NUM))
> +  for (list = OMP_LIST_ENTER; list != OMP_LIST_NUM;
> +       list = (list == OMP_LIST_ENTER
> +	       ? OMP_LIST_TO
> +	       : (list == OMP_LIST_TO ? OMP_LIST_LINK : OMP_LIST_NUM)))
>      for (n = c->lists[list]; n; n = n->next)
>        if (n->sym)
>  	{
> @@ -4564,14 +4584,14 @@ gfc_match_omp_declare_target (void)
>  		   && n->sym->attr.omp_declare_target_link
>  		   && list != OMP_LIST_LINK)
>  	    gfc_error_now ("OMP DECLARE TARGET variable at %L previously "
> -			   "mentioned in LINK clause and later in TO clause",
> -			   &n->where);
> +			   "mentioned in LINK clause and later in %s clause",
> +			   &n->where, list == OMP_LIST_TO ? "TO" : "ENTER");
>  	  else if (n->sym->attr.omp_declare_target
>  		   && !n->sym->attr.omp_declare_target_link
>  		   && list == OMP_LIST_LINK)
>  	    gfc_error_now ("OMP DECLARE TARGET variable at %L previously "
> -			   "mentioned in TO clause and later in LINK clause",
> -			   &n->where);
> +			   "mentioned in TO or ENTER clause and later in "
> +			   "LINK clause", &n->where);

The wording of the messages "previously mentioned in FOO and later in BAR clause"
makes not much sense to me, because we in the Fortran FE don't remember which
clause was specified earlier and which one was later.
The loop is walking first all enter clauses, then all to clauses, then all
link clauses.

Now, if we change the wording so that it complains that a variable is
"mentioned in both FOO and BAR clauses", we could avoid the TO or ENTER
stuff even without some O(n^2) complexity or extra bit somewhere simply
by walking the clauses in the order of TO, LINK, ENTER (or ENTER, LINK, TO)
clauses, wer could be exact.

Though, further thinking about it, this isn't just about the case
where it is on the same declare target, but also on multiple and in that
case previous/later makes sense.

As we don't remember if it was previously TO or ENTER, I'd just suggest:
1) simplify the 2 for cycles, with 3 lists it is too unreadable, so use
   something like:
  static const int to_enter_link_lists[]
    = { OMP_LIST_TO, OMP_LIST_ENTER, OMP_LIST_LINK };
  for (int listn = 0; listn < ARRAY_SIZE (to_enter_link_lists)
		      && (list = to_enter_link_lists[listn], true); ++listn)
2) move the
          else if (n->sym->mark)
            gfc_error_now ("Variable at %L mentioned multiple times in "
                           "clauses of the same OMP DECLARE TARGET directive",
                           &n->where);
  diagnostics earlier, above
          else if (n->sym->attr.omp_declare_target
                   && n->sym->attr.omp_declare_target_link
                   && list != OMP_LIST_LINK)
  and adjust testsuite if needed

Ok with those changes.

On the C/C++ FE side, I'll need to change %<to%> to %<to%> or %<enter%>.

	Jakub


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Patch] OpenMP/Fortran: Add support for enter clause on declare target (was: [committed] openmp: Add support for enter clause on declare target)
  2022-05-27 15:20   ` Jakub Jelinek
@ 2022-05-27 15:44     ` Jakub Jelinek
  0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2022-05-27 15:44 UTC (permalink / raw)
  To: Tobias Burnus, gcc-patches, fortran

On Fri, May 27, 2022 at 05:20:08PM +0200, Jakub Jelinek wrote:
> 2) move the
>           else if (n->sym->mark)
>             gfc_error_now ("Variable at %L mentioned multiple times in "
>                            "clauses of the same OMP DECLARE TARGET directive",
>                            &n->where);
>   diagnostics earlier, above
>           else if (n->sym->attr.omp_declare_target
>                    && n->sym->attr.omp_declare_target_link
>                    && list != OMP_LIST_LINK)
>   and adjust testsuite if needed

Note, that matches what the C/C++ FEs do, they also first check for clauses
on the same construct and only afterwards do the link vs. to/enter on
separate directives.

Here is an incremental patch I plan to commit after full testing for the
C/C++ FE wording.

2022-05-27  Jakub Jelinek  <jakub@redhat.com>

gcc/c/
	* c-parser.cc (c_parser_omp_declare_target): If OMP_CLAUSE_LINK was
	seen first, use "%<to%>" or "%<enter%>" depending on
	OMP_CLAUSE_ENTER_TO of the current clause, otherwise use
	"%<to%> or %<enter%>" wording.
gcc/cp/
	* parser.c (handle_omp_declare_target_clause): If OMP_CLAUSE_LINK was
	seen first, use "%<to%>" or "%<enter%>" depending on
	OMP_CLAUSE_ENTER_TO of the current clause, otherwise use
	"%<to%> or %<enter%>" wording.
gcc/testsuite/
	* c-c++-common/gomp/declare-target-2.c: Add further tests for mixing of
	link and to/enter clauses on separate directives.

--- gcc/c/c-parser.cc.jj	2022-05-26 23:22:30.312850583 +0200
+++ gcc/c/c-parser.cc	2022-05-27 17:28:59.120420300 +0200
@@ -22067,9 +22067,14 @@ c_parser_omp_declare_target (c_parser *p
 	id = get_identifier ("omp declare target");
       if (at2)
 	{
-	  error_at (OMP_CLAUSE_LOCATION (c),
-		    "%qD specified both in declare target %<link%> and %<to%>"
-		    " clauses", t);
+	  if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_ENTER)
+	    error_at (OMP_CLAUSE_LOCATION (c),
+		      "%qD specified both in declare target %<link%> and %qs"
+		      " clauses", t, OMP_CLAUSE_ENTER_TO (c) ? "to" : "enter");
+	  else
+	    error_at (OMP_CLAUSE_LOCATION (c),
+		      "%qD specified both in declare target %<link%> and "
+		      "%<to%> or %<enter%> clauses", t);
 	  continue;
 	}
       if (!at1)
--- gcc/cp/parser.cc.jj	2022-05-26 23:22:30.306850645 +0200
+++ gcc/cp/parser.cc	2022-05-27 17:31:19.086980582 +0200
@@ -45991,9 +45991,14 @@ handle_omp_declare_target_clause (tree c
     id = get_identifier ("omp declare target");
   if (at2)
     {
-      error_at (OMP_CLAUSE_LOCATION (c),
-		"%qD specified both in declare target %<link%> and %<to%>"
-		" clauses", t);
+      if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_ENTER)
+	error_at (OMP_CLAUSE_LOCATION (c),
+		  "%qD specified both in declare target %<link%> and %qs"
+		  " clauses", t, OMP_CLAUSE_ENTER_TO (c) ? "to" : "enter");
+      else
+	error_at (OMP_CLAUSE_LOCATION (c),
+		  "%qD specified both in declare target %<link%> and "
+		  "%<to%> or %<enter%> clauses", t);
       return false;
     }
   if (!at1)
--- gcc/testsuite/c-c++-common/gomp/declare-target-2.c.jj	2022-05-27 12:31:36.114087022 +0200
+++ gcc/testsuite/c-c++-common/gomp/declare-target-2.c	2022-05-27 17:38:12.173731493 +0200
@@ -10,7 +10,22 @@ int b;
 #pragma omp declare target enter (b) link (b)	/* { dg-error "appears more than once on the same .declare target. directive" } */
 int c;
 #pragma omp declare target (c)
-#pragma omp declare target link (c)		/* { dg-error "specified both in declare target" } */
+#pragma omp declare target link (c)		/* { dg-error "specified both in declare target 'link' and 'to' or 'enter' clauses" } */
+int c2;
+#pragma omp declare target to (c2)
+#pragma omp declare target link (c2)		/* { dg-error "specified both in declare target 'link' and 'to' or 'enter' clauses" } */
+int c3;
+#pragma omp declare target enter (c3)
+#pragma omp declare target link (c3)		/* { dg-error "specified both in declare target 'link' and 'to' or 'enter' clauses" } */
+int c4;
+#pragma omp declare target link (c4)
+#pragma omp declare target (c4)			/* { dg-error "specified both in declare target 'link' and 'enter' clauses" } */
+int c5;
+#pragma omp declare target link (c5)
+#pragma omp declare target to (c5)		/* { dg-error "specified both in declare target 'link' and 'to' clauses" } */
+int c6;
+#pragma omp declare target link (c6)
+#pragma omp declare target enter (c6)		/* { dg-error "specified both in declare target 'link' and 'enter' clauses" } */
 int foo (void);
 #pragma omp declare target link (foo)		/* { dg-error "is not a variable in clause" } */
 struct S;

	Jakub


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-05-27 15:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <YpCt2rJGuydcFlA4@tucnak>
2022-05-27 14:52 ` [Patch] OpenMP/Fortran: Add support for enter clause on declare target (was: [committed] openmp: Add support for enter clause on declare target) Tobias Burnus
2022-05-27 15:20   ` Jakub Jelinek
2022-05-27 15:44     ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).