public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch] OpenMP: Accept argument to depobj's destroy clause
@ 2023-11-23 14:21 Tobias Burnus
  2023-11-23 14:32 ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Tobias Burnus @ 2023-11-23 14:21 UTC (permalink / raw)
  To: gcc-patches, fortran

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

I stumbled over this trivial omission which blocks some testcases.

I am not sure whether I have solved the is-same-expr most elegantly,
but I did loosely follow the duplicated-entry check for 'map'. As that's
a restriction to the user, we don't have to catch all and I hope the code
catches the most important violations, doesn't ICE and does not reject
valid code. At least for all real-world code it should™ work, but I
guess for lvalue expressions involving function calls it probably doesn't.

Thoughts, comments?

Tobias

PS: GCC accepts an lvalue expression in C/C++ and only a identifier
for a scalar variable in Fortran, i.e. neither array elements nor
structure components.

Which variant is right depends whether one reads OpenMP 5.1 (lvalue expr,
scalar variable) or 5.2 (variable without permitting array sections or
structure components) - whereas TR12 has the same but talks about
locator list items in one restriction. For the OpenMP mess, see spec
issue #3739.
-----------------
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: depobj-destroy.diff --]
[-- Type: text/x-patch, Size: 10566 bytes --]

OpenMP: Accept argument to depobj's destroy clause

Since OpenMP 5.2, the destroy clause takes an depend argument as argument;
for the depobj directive, it the new argument is optional but, if present,
it must be identical to the directive's argument.

gcc/c/ChangeLog:

	* c-parser.cc (c_parser_omp_depobj): Accept optionally an argument
	to the destroy clause.

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_omp_depobj): Accept optionally an argument
	to the destroy clause.

gcc/fortran/ChangeLog:

	* openmp.cc (gfc_match_omp_depobj): Accept optionally an argument
	to the destroy clause.

libgomp/ChangeLog:

	* libgomp.texi (5.2 Impl. Status): An argument to the destroy clause
	is now supported.

gcc/testsuite/ChangeLog:

	* c-c++-common/gomp/depobj-3.c: New test.
	* gfortran.dg/gomp/depobj-3.f90: New test.

 gcc/c/c-parser.cc                           | 57 ++++++++++++++++++++++++++-
 gcc/cp/parser.cc                            | 60 ++++++++++++++++++++++++++++-
 gcc/fortran/openmp.cc                       | 15 +++++++-
 gcc/testsuite/c-c++-common/gomp/depobj-3.c  | 40 +++++++++++++++++++
 gcc/testsuite/gfortran.dg/gomp/depobj-3.f90 | 18 +++++++++
 libgomp/libgomp.texi                        |  2 +-
 6 files changed, 188 insertions(+), 4 deletions(-)

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 371dd29557b..378647c1a67 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -21605,6 +21605,9 @@ c_parser_omp_critical (location_t loc, c_parser *parser, bool *if_p)
      destroy
      update (dependence-type)
 
+   OpenMP 5.2 additionally:
+     destroy ( depobj )
+
    dependence-type:
      in
      out
@@ -21663,7 +21666,59 @@ c_parser_omp_depobj (c_parser *parser)
 	    clause = error_mark_node;
 	}
       else if (!strcmp ("destroy", p))
-	kind = OMP_CLAUSE_DEPEND_LAST;
+	{
+	  matching_parens c_parens;
+	  kind = OMP_CLAUSE_DEPEND_LAST;
+	  if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)
+	      && c_parens.require_open (parser))
+	    {
+	      tree destobj = c_parser_expr_no_commas (parser, NULL).value;
+	      /* OpenMP requires that the two expressions are identical; catch
+		 the most common mismatches.  */
+	      if (!lvalue_p (destobj))
+		error_at (EXPR_LOC_OR_LOC (destobj, c_loc),
+			  "%<destrory%> expression is not lvalue expression");
+	      else if (depobj != error_mark_node)
+		{
+		  tree t = depobj;
+		  tree t2 = build_unary_op (EXPR_LOC_OR_LOC (destobj, c_loc),
+					    ADDR_EXPR, destobj, false);
+		  if (t2 != error_mark_node)
+		    t2 = build_indirect_ref (EXPR_LOC_OR_LOC (t2, c_loc),
+					     t2, RO_UNARY_STAR);
+		  while (TREE_CODE (t) == COMPONENT_REF
+			 || TREE_CODE (t) == ARRAY_REF)
+                    {
+		      t = TREE_OPERAND (t, 0);
+		      if (TREE_CODE (t) == MEM_REF || INDIRECT_REF_P (t))
+			{
+			  t = TREE_OPERAND (t, 0);
+			  STRIP_NOPS (t);
+			  if (TREE_CODE (t) == POINTER_PLUS_EXPR)
+			    t = TREE_OPERAND (t, 0);
+                        }
+		    }
+		  while (TREE_CODE (t2) == COMPONENT_REF
+			 || TREE_CODE (t2) == ARRAY_REF)
+                    {
+		      t2 = TREE_OPERAND (t2, 0);
+		      if (TREE_CODE (t2) == MEM_REF || INDIRECT_REF_P (t2))
+			{
+			  t2 = TREE_OPERAND (t2, 0);
+			  STRIP_NOPS (t2);
+			  if (TREE_CODE (t2) == POINTER_PLUS_EXPR)
+			    t2 = TREE_OPERAND (t2, 0);
+                        }
+		    }
+		  if (DECL_UID (t) != DECL_UID (t2))
+		    error_at (EXPR_LOC_OR_LOC (destobj, c_loc),
+			      "the %<destroy%> expression %qE must be the same "
+			      "as the %<depobj%> argument %qE",
+			      destobj, depobj);
+		}
+	      c_parens.skip_until_found_close (parser);
+	    }
+	}
       else if (!strcmp ("update", p))
 	{
 	  matching_parens c_parens;
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index f6d088bc73f..0fff66555bf 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -43173,6 +43173,9 @@ cp_parser_omp_critical (cp_parser *parser, cp_token *pragma_tok, bool *if_p)
      destroy
      update (dependence-type)
 
+   OpenMP 5.2 additionally:
+     destroy ( depobj )
+
    dependence-type:
      in
      out
@@ -43219,7 +43222,62 @@ cp_parser_omp_depobj (cp_parser *parser, cp_token *pragma_tok)
 	    clause = error_mark_node;
 	}
       else if (!strcmp ("destroy", p))
-	kind = OMP_CLAUSE_DEPEND_LAST;
+	{
+	  kind = OMP_CLAUSE_DEPEND_LAST;
+	  matching_parens c_parens;
+	  if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_PAREN)
+	      && c_parens.require_open (parser))
+	    {
+	      tree destobj = cp_parser_assignment_expression (parser);
+	      /* OpenMP requires that the two expressions are identical; catch
+		 the most common mismatches.  */
+	      if (depobj != error_mark_node && destobj != error_mark_node)
+		{
+		  tree t = depobj;
+		  tree t2 = destobj;
+		  while (TREE_CODE (t) == COMPONENT_REF
+			 || TREE_CODE (t) == ARRAY_REF
+			 || TREE_CODE (t) == VIEW_CONVERT_EXPR)
+		    {
+		      t = TREE_OPERAND (t, 0);
+		      if (REFERENCE_REF_P (t))
+			t = TREE_OPERAND (t, 0);
+		      if (TREE_CODE (t) == MEM_REF || INDIRECT_REF_P (t))
+			{
+			  t = TREE_OPERAND (t, 0);
+			  STRIP_NOPS (t);
+			  if (TREE_CODE (t) == POINTER_PLUS_EXPR)
+			    t = TREE_OPERAND (t, 0);
+			}
+		    }
+		  while (TREE_CODE (t2) == COMPONENT_REF
+			 || TREE_CODE (t2) == ARRAY_REF
+			 || TREE_CODE (t2) == VIEW_CONVERT_EXPR)
+		    {
+		      t2 = TREE_OPERAND (t2, 0);
+		      if (REFERENCE_REF_P (t2))
+			t2 = TREE_OPERAND (t2, 0);
+		      if (TREE_CODE (t2) == MEM_REF || INDIRECT_REF_P (t2))
+			{
+			  t2 = TREE_OPERAND (t2, 0);
+			  STRIP_NOPS (t2);
+			  if (TREE_CODE (t2) == POINTER_PLUS_EXPR)
+			    t2 = TREE_OPERAND (t2, 0);
+			}
+		    }
+		  if (t != t2)
+		    error_at (EXPR_LOC_OR_LOC (destobj, c_loc),
+			      "the %<destroy%> expression %qE must be the same "
+			      "as the %<depobj%> argument %qE",
+			      destobj, depobj);
+		}
+	      if (!c_parens.require_close (parser))
+		cp_parser_skip_to_closing_parenthesis (parser,
+						       /*recovering=*/true,
+						       /*or_comma=*/false,
+						       /*consume_paren=*/true);
+	    }
+	}
       else if (!strcmp ("update", p))
 	{
 	  matching_parens c_parens;
diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index 2e2e23d567b..5f37f3a2586 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -4731,10 +4731,23 @@ gfc_match_omp_depobj (void)
 	  goto error;
 	}
     }
-  else if (gfc_match ("destroy") == MATCH_YES)
+  else if (gfc_match ("destroy ") == MATCH_YES)
     {
+      gfc_expr *destroyobj = NULL;
       c = gfc_get_omp_clauses ();
       c->destroy = true;
+
+      if (gfc_match (" ( %v ) ", &destroyobj) == MATCH_YES)
+	{
+	  if (destroyobj->symtree != depobj->symtree)
+	    {
+	      gfc_error ("The same depend object must be used as DEPOBJ argument at %L"
+			 " and as DESTROY argument at %L", &depobj->where,
+			 &destroyobj->where);
+	      return MATCH_ERROR;
+	    }
+	  gfc_free_expr (destroyobj);
+	}
     }
   else if (gfc_match_omp_clauses (&c, omp_mask (OMP_CLAUSE_DEPEND), true, false)
 	   != MATCH_YES)
diff --git a/gcc/testsuite/c-c++-common/gomp/depobj-3.c b/gcc/testsuite/c-c++-common/gomp/depobj-3.c
new file mode 100644
index 00000000000..27c66ed5019
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/gomp/depobj-3.c
@@ -0,0 +1,40 @@
+typedef struct __attribute__((__aligned__ (sizeof (void *)))) omp_depend_t {
+  char __omp_depend_t__[2 * sizeof (void *)];
+} omp_depend_t;
+
+void
+f ()
+{
+  omp_depend_t obj2;
+  struct { omp_depend_t c; } s;
+  float a;
+  #pragma omp depobj(s.c) depend(inout: a)
+
+  #pragma omp depobj(s.c) destroy(s.c) /* OK */
+
+  #pragma omp depobj(s.c) destroy(obj2)
+/* { dg-error "the 'destroy' expression 'obj2' must be the same as the 'depobj' argument 's.c'" "" { target c } .-1 } */
+/* { dg-error "the 'destroy' expression 'obj2' must be the same as the 'depobj' argument 's.f\\(\\)::<unnamed struct>::c'" "" { target c++ } .-2 } */
+}
+
+int
+main ()
+{
+   float a;
+   omp_depend_t obj;
+
+   #pragma omp depobj(obj) depend(inout: a)
+
+   #pragma omp depobj(obj) destroy(obj) /* OK */
+
+   #pragma omp depobj(obj) destroy(a + 5) 
+/* { dg-error "'destrory' expression is not lvalue expression" "" { target c } .-1 } */
+/* { dg-error "the 'destroy' expression '\\(a \\+ \\(float\\)5\\)' must be the same as the 'depobj' argument 'obj'" "" { target c++ } .-2 } */
+
+   #pragma omp depobj(obj+5) destroy(a) 
+/* { dg-error "invalid operands to binary \\+ \\(have 'omp_depend_t' and 'int'\\)" "" { target c } .-1 } */
+/* { dg-error "no match for 'operator\\+' in 'obj \\+ 5' \\(operand types are 'omp_depend_t' and 'int'\\)" "" { target c++ } .-2 } */
+
+   #pragma omp depobj(obj) destroy(a)  /* { dg-error "the 'destroy' expression 'a' must be the same as the 'depobj' argument 'obj'" } */
+   return 0;
+}
diff --git a/gcc/testsuite/gfortran.dg/gomp/depobj-3.f90 b/gcc/testsuite/gfortran.dg/gomp/depobj-3.f90
new file mode 100644
index 00000000000..a0020014f9e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/depobj-3.f90
@@ -0,0 +1,18 @@
+! { dg-do compile { target { fortran_integer_16 || ilp32 } } }
+! omp_depend_kind = 2*intptr_t --> 16 (128 bit) on 64bit-pointer systems
+!                              --> 8  (128 bit) on 32bit-pointer systems
+subroutine f1
+  !use omp_lib   ! N/A in gcc/testsuite
+  use iso_c_binding, only: c_intptr_t
+  implicit none
+  integer, parameter :: omp_depend_kind = 2*c_intptr_t
+  integer :: a, b
+  integer(kind=omp_depend_kind) :: depobj, depobj1(5)
+
+  !$omp depobj(depobj) destroy
+
+  !$omp depobj(depobj) destroy( depobj)
+
+  !$omp depobj(depobj) destroy( depobj2)  ! { dg-error "The same depend object must be used as DEPOBJ argument at .1. and as DESTROY argument at .2." }
+  !$omp depobj(depobj) destroy( a)  ! { dg-error "The same depend object must be used as DEPOBJ argument at .1. and as DESTROY argument at .2." }
+end
diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi
index 2f6227c94b2..e5fe7af76af 100644
--- a/libgomp/libgomp.texi
+++ b/libgomp/libgomp.texi
@@ -387,7 +387,7 @@ to address of matching mapped list item per 5.1, Sect. 2.21.7.2 @tab N @tab
       @code{-Wall}).  Unknown clauses are always rejected with an error.}
 @item Clauses on @code{end} directive can be on directive @tab Y @tab
 @item @code{destroy} clause with destroy-var argument on @code{depobj}
-      @tab N @tab
+      @tab Y @tab
 @item Deprecation of no-argument @code{destroy} clause on @code{depobj}
       @tab N @tab
 @item @code{linear} clause syntax changes and @code{step} modifier @tab Y @tab

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

end of thread, other threads:[~2023-11-24 12:24 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-23 14:21 [Patch] OpenMP: Accept argument to depobj's destroy clause Tobias Burnus
2023-11-23 14:32 ` Jakub Jelinek
2023-11-23 15:21   ` Tobias Burnus
2023-11-23 15:32     ` Jakub Jelinek
2023-11-23 15:59       ` Tobias Burnus
2023-11-23 16:46         ` Jakub Jelinek
2023-11-24 12:24           ` [Patch,v3] " Tobias Burnus

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).