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

* Re: [Patch] OpenMP: Accept argument to depobj's destroy clause
  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
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2023-11-23 14:32 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: gcc-patches, fortran

On Thu, Nov 23, 2023 at 03:21:41PM +0100, Tobias Burnus wrote:
> 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

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

Please watch indentation, seems there is a mix of 8 spaces vs. tabs:

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

Nothing checks that t and t2 here are decls.  Use operand_equal_p instead?

> +		    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);
> +	    }
> +	}
> +		  if (t != t2)

And here too?

	Jakub


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

* Re: [Patch] OpenMP: Accept argument to depobj's destroy clause
  2023-11-23 14:32 ` Jakub Jelinek
@ 2023-11-23 15:21   ` Tobias Burnus
  2023-11-23 15:32     ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Tobias Burnus @ 2023-11-23 15:21 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, fortran

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

Hi Jakub,

On 23.11.23 15:32, Jakub Jelinek wrote:
> On Thu, Nov 23, 2023 at 03:21:41PM +0100, Tobias Burnus wrote:
>> I stumbled over this trivial omission which blocks some testcases.
>> I am not sure whether I have solved the is-same-expr most elegantly,
Answer: I didn't - as expected.
> + if (DECL_UID (t) != DECL_UID (t2))
> Nothing checks that t and t2 here are decls.  Use operand_equal_p instead?

Yes – I think I can simply use this instead all the other checks. That is
the function I was looking for but couldn't find before.

(I even have use the function before for PR108545.)

I decided that volatileness is fine and using twice a volatile function is
Okay according to the spec - hence, I permit this in addition. (One can
argue about it - but as specifying both is mandatory in OpenMP 6.0, it
seems to make sense.)

Revised version attached.

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: depobj-destroy-v2.diff --]
[-- Type: text/x-patch, Size: 8510 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                           | 23 +++++++++++++-
 gcc/cp/parser.cc                            | 24 ++++++++++++++-
 gcc/fortran/openmp.cc                       | 15 ++++++++-
 gcc/testsuite/c-c++-common/gomp/depobj-3.c  | 47 +++++++++++++++++++++++++++++
 gcc/testsuite/gfortran.dg/gomp/depobj-3.f90 | 18 +++++++++++
 libgomp/libgomp.texi                        |  2 +-
 6 files changed, 125 insertions(+), 4 deletions(-)

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 371dd29557b..006aee3e93f 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,25 @@ 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;
+	      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
+		       && !operand_equal_p (destobj, depobj,
+					    OEP_MATCH_SIDE_EFFECTS))
+		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..1fca6bff795 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,26 @@ 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);
+	      if (depobj != error_mark_node
+		  && destobj != error_mark_node
+		  && !operand_equal_p (destobj, depobj, OEP_MATCH_SIDE_EFFECTS))
+		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..2df66f4ebf1
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/gomp/depobj-3.c
@@ -0,0 +1,47 @@
+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 } */
+}
+
+void
+g ()
+{
+  volatile omp_depend_t obj3;
+  #pragma omp depobj(obj3) destroy(obj3)
+}
+
+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

* Re: [Patch] OpenMP: Accept argument to depobj's destroy clause
  2023-11-23 15:21   ` Tobias Burnus
@ 2023-11-23 15:32     ` Jakub Jelinek
  2023-11-23 15:59       ` Tobias Burnus
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2023-11-23 15:32 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: gcc-patches, fortran

On Thu, Nov 23, 2023 at 04:21:50PM +0100, Tobias Burnus wrote:
> @@ -21663,7 +21666,25 @@ 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;
> +	      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
> +		       && !operand_equal_p (destobj, depobj,
> +					    OEP_MATCH_SIDE_EFFECTS))

There is also OEP_LEXICOGRAPHIC which could be used in addition to that.
The question is if we want to consider say
#pragma depobj (a[++i]) destroy (a[++i])
as same or different (similarly a[foo ()] in both cases).
A function could at least in theory return the same value, for other
side-effects there is some wiggle room in unspecified number of times how
many the side-effects of clauses are evaluated (and for destroy we really
don't intend to evaluate them at all for the clause, just for the directive
argument).

	Jakub


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

* Re: [Patch] OpenMP: Accept argument to depobj's destroy clause
  2023-11-23 15:32     ` Jakub Jelinek
@ 2023-11-23 15:59       ` Tobias Burnus
  2023-11-23 16:46         ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Tobias Burnus @ 2023-11-23 15:59 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, fortran

Hi Jakub,

On 23.11.23 16:32, Jakub Jelinek wrote:
> On Thu, Nov 23, 2023 at 04:21:50PM +0100, Tobias Burnus wrote:
>> @@ -21663,7 +21666,25 @@ c_parser_omp_depobj (c_parser *parser)
>> +          else if (depobj != error_mark_node
>> +                   && !operand_equal_p (destobj, depobj,
>> +                                        OEP_MATCH_SIDE_EFFECTS))
> There is also OEP_LEXICOGRAPHIC which could be used in addition to that.
> The question is if we want to consider say
> #pragma depobj (a[++i]) destroy (a[++i])
> as same or different (similarly a[foo ()] in both cases).

I don't think that we want to permit those; I think there is (a) the
question whether both expressions have to be evaluated or not and (b),
if so, in which order and (c), if the run-time result is different,
whether both have to be 'destory'ed or only one of them (which one?).

Additionally, 'destroy-var must refer to the same depend object as the
depobj argument of the construct.' cannot be fulfilled if one is
evaluated before the other and both use the same 'i' in your case.

Thus, I do not really see an argument for permitting OEP_LEXICOGRAPHIC.

I think permitting 'volatile' does make sense, in a ways, as a
hyper-careful user might actually write such code.

[I wonder whether the OpenMP wording would permit 'omp depobj(obj)
destroy(f())' with 'auto f() { return obj; }' – but I am sure we don't
want to permit it in the compiler.]

Tobias

PS: In any case, I find it confusing to require that the same
variable/lvalue-expression has to be specified twice. The (only) pro is
that for 'omp interop destroy(...)' the argument is required and for
consistency of the 'destroy' clause, an argument now must be (always)
specified. But that leads to the odd 'omp depobj(obj) destroy(obj)',
which is really ugly. (In 5.2 the arg to destroy is optional but
omitting it is deprecated; hence, in OpenMP 6.0 (TR11, TR12) the
argument must be specified twice.)

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

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

* Re: [Patch] OpenMP: Accept argument to depobj's destroy clause
  2023-11-23 15:59       ` Tobias Burnus
@ 2023-11-23 16:46         ` Jakub Jelinek
  2023-11-24 12:24           ` [Patch,v3] " Tobias Burnus
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2023-11-23 16:46 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: gcc-patches, fortran

On Thu, Nov 23, 2023 at 04:59:16PM +0100, Tobias Burnus wrote:
> > There is also OEP_LEXICOGRAPHIC which could be used in addition to that.
> > The question is if we want to consider say
> > #pragma depobj (a[++i]) destroy (a[++i])
> > as same or different (similarly a[foo ()] in both cases).
> 
> I don't think that we want to permit those; I think there is (a) the
> question whether both expressions have to be evaluated or not and (b),
> if so, in which order and (c), if the run-time result is different,
> whether both have to be 'destory'ed or only one of them (which one?).

Well, we don't need to destroy two, because it would be UB if the two
aren't the same.  This is just about diagnostics if user messed stuff
up unintentionally.
The function call case can be the same very easily, just
int foo () { return 0; }
omp_depend_t a[2];
...
#pragma omp depobj (a[foo ()]) destroy (a[foo ()])
or
int i = 0;
#pragma omp depobj (a[((++i) * 2) & 1]) destroy (a[((++i) * 2) & 1])
The former may evaluate the function call multiple times, but user arranges
for it to do the same thing in each case, in the second case while there
are side-effects, they don't really matter for the value, just in whether
i after this pragma has value of 0, 1, 2 or something else (but if again
nothing cares about that value afterwards...).

The question is if same (I admit I haven't looked up the exact wording now)
means lexically same, or anything that has the same value, etc.
Because e.g.
omp_depend_t a;
...
omp_depend_t *p = &a;
#pragma omp depobj (a) destroy (p[0])
is the same value but not lexically same.

IMHO the argument to destroy clause shouldn't have ever been allowed, it is
only unnecessary extra pain.

	Jakub


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

* [Patch,v3] OpenMP: Accept argument to depobj's destroy clause
  2023-11-23 16:46         ` Jakub Jelinek
@ 2023-11-24 12:24           ` Tobias Burnus
  0 siblings, 0 replies; 7+ messages in thread
From: Tobias Burnus @ 2023-11-24 12:24 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, fortran

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

As discussed on IRC, we now go for a warning and useOEP_LEXICOGRAPHIC - at least until the spec issue has been solve.

[The OpenMP spec Issue 3739 tracks the open questions/issues mentioned
in this thread.] Updated patch attached. 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: depobj-destroy-v3.diff --]
[-- Type: text/x-patch, Size: 8582 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                           | 24 ++++++++++++++-
 gcc/cp/parser.cc                            | 25 ++++++++++++++-
 gcc/fortran/openmp.cc                       | 12 +++++++-
 gcc/testsuite/c-c++-common/gomp/depobj-3.c  | 47 +++++++++++++++++++++++++++++
 gcc/testsuite/gfortran.dg/gomp/depobj-3.f90 | 18 +++++++++++
 libgomp/libgomp.texi                        |  2 +-
 6 files changed, 124 insertions(+), 4 deletions(-)

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 371dd29557b..989c0503f37 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,26 @@ 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;
+	      if (!lvalue_p (destobj))
+		error_at (EXPR_LOC_OR_LOC (destobj, c_loc),
+			  "%<destroy%> expression is not lvalue expression");
+	      else if (depobj != error_mark_node
+		       && !operand_equal_p (destobj, depobj,
+					    OEP_MATCH_SIDE_EFFECTS
+					    | OEP_LEXICOGRAPHIC))
+		warning_at (EXPR_LOC_OR_LOC (destobj, c_loc), 0,
+			    "the %<destroy%> expression %qE should 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..e4e2feac486 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,27 @@ 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);
+	      if (depobj != error_mark_node
+		  && destobj != error_mark_node
+		  && !operand_equal_p (destobj, depobj, OEP_MATCH_SIDE_EFFECTS
+							| OEP_LEXICOGRAPHIC))
+		warning_at (EXPR_LOC_OR_LOC (destobj, c_loc), 0,
+			    "the %<destroy%> expression %qE should 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..b9ac61103af 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -4731,10 +4731,20 @@ 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_warning (0, "The same depend object should be used as DEPOBJ "
+			 "argument at %L and as DESTROY argument at %L",
+			 &depobj->where, &destroyobj->where);
+	  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..a5017a40b47
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/gomp/depobj-3.c
@@ -0,0 +1,47 @@
+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-warning "the 'destroy' expression 'obj2' should be the same as the 'depobj' argument 's.c'" "" { target c } .-1 } */
+/* { dg-warning "the 'destroy' expression 'obj2' should be the same as the 'depobj' argument 's.f\\(\\)::<unnamed struct>::c'" "" { target c++ } .-2 } */
+}
+
+void
+g ()
+{
+  volatile omp_depend_t obj3;
+  #pragma omp depobj(obj3) destroy(obj3)
+}
+
+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 "'destroy' expression is not lvalue expression" "" { target c } .-1 } */
+/* { dg-warning "the 'destroy' expression '\\(a \\+ \\(float\\)5\\)' should 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-warning "the 'destroy' expression 'a' should 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..8a3625e8883
--- /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), depobj2
+
+  !$omp depobj(depobj) destroy
+
+  !$omp depobj(depobj) destroy( depobj)
+
+  !$omp depobj(depobj) destroy( depobj2)  ! { dg-warning "The same depend object should be used as DEPOBJ argument at .1. and as DESTROY argument at .2." }
+  !$omp depobj(depobj) destroy( a)  ! { dg-warning "The same depend object should 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).