[PATCH] OpenMP front-end: allow requires dynamic_allocators

[PATCH] OpenMP front-end: allow requires dynamic_allocators

[Andrew Stubbs]

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

Hi all,

This patch removes the "sorry" message for the OpenMP "requires 
dynamic_allocators" feature in C, C++ and Fortran.

The clause is supposed to state that the user code will not work without 
the omp_alloc/omp_free and omp_init_allocator/omp_destroy_allocator and 
these things *are* present, so there should be no problem allowing it.

I can't see any reason for our implementation to *do* anything with this 
statement -- it's fine for the allocators to work the same with or 
without it.

I think this patch ought to be fine for GCC 12, but if not it can wait 
until stage 1 opens. I shall backport this to OG11 shortly.

OK?

Andrew

[-- Attachment #2: 211220-requires-dynamic_allocators.patch --]
[-- Type: text/plain, Size: 2573 bytes --]

OpenMP: allow requires dynamic_allocators

There's no need to reject the dynamic_allocators requires directive because
we actually do support the feature, and it doesn't have to actually "do"
anything.

gcc/c/ChangeLog:

	* c-parser.c (c_parser_omp_requires): Don't "sorry" dynamic_allocators.

gcc/cp/ChangeLog:

	* parser.c (cp_parser_omp_requires): Don't "sorry" dynamic_allocators.

gcc/fortran/ChangeLog:

	* openmp.c (gfc_match_omp_requires): Don't "sorry" dynamic_allocators.

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/requires-8.f90: Reinstate dynamic allocators
	requirement.

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index d7e5f051ac0..808a73f1038 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -22581,7 +22581,7 @@ c_parser_omp_requires (c_parser *parser)
 	      c_parser_skip_to_pragma_eol (parser, false);
 	      return;
 	    }
-	  if (p)
+	  if (p && this_req != OMP_REQUIRES_DYNAMIC_ALLOCATORS)
 	    sorry_at (cloc, "%qs clause on %<requires%> directive not "
 			    "supported yet", p);
 	  if (p)
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index c2564e51e41..d55c9675c4e 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -46421,7 +46421,7 @@ cp_parser_omp_requires (cp_parser *parser, cp_token *pragma_tok)
 	      cp_parser_skip_to_pragma_eol (parser, pragma_tok);
 	      return false;
 	    }
-	  if (p)
+	  if (p && this_req != OMP_REQUIRES_DYNAMIC_ALLOCATORS)
 	    sorry_at (cloc, "%qs clause on %<requires%> directive not "
 			    "supported yet", p);
 	  if (p)
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 2036bc1349f..f47a44f7539 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -5373,7 +5373,8 @@ gfc_match_omp_requires (void)
       else
 	goto error;
 
-      if (requires_clause & ~OMP_REQ_ATOMIC_MEM_ORDER_MASK)
+      if (requires_clause & ~(OMP_REQ_ATOMIC_MEM_ORDER_MASK
+			      | OMP_REQ_DYNAMIC_ALLOCATORS))
 	gfc_error_now ("Sorry, %qs clause at %L on REQUIRES directive is not "
 		       "yet supported", clause, &old_loc);
       if (!gfc_omp_requires_add_clause (requires_clause, clause, &old_loc, NULL))
diff --git a/gcc/testsuite/gfortran.dg/gomp/requires-8.f90 b/gcc/testsuite/gfortran.dg/gomp/requires-8.f90
index 3c32ae9860e..eadfcaf8609 100644
--- a/gcc/testsuite/gfortran.dg/gomp/requires-8.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/requires-8.f90
@@ -4,7 +4,7 @@ contains
  subroutine foo
   interface
    subroutine bar2
-     !$!omp requires dynamic_allocators
+     !$omp requires dynamic_allocators
    end subroutine
   end interface
   !$omp target

Re: [PATCH] OpenMP front-end: allow requires dynamic_allocators

[Jakub Jelinek]

On Mon, Dec 20, 2021 at 03:16:23PM +0000, Andrew Stubbs wrote:
> This patch removes the "sorry" message for the OpenMP "requires
> dynamic_allocators" feature in C, C++ and Fortran.
> 
> The clause is supposed to state that the user code will not work without the
> omp_alloc/omp_free and omp_init_allocator/omp_destroy_allocator and these
> things *are* present, so there should be no problem allowing it.
> 
> I can't see any reason for our implementation to *do* anything with this
> statement -- it's fine for the allocators to work the same with or without
> it.

Well, we should do a lot with it, but that can wait for later.
In particular, if OMP_REQUIRES_DYNAMIC_ALLOCATORS is not present,
we should be rejecting omp_init_allocator and omp_destroy_allocator
in target regions (maybe just a warning though), and for allocate clauses,
directives, and the omp_alloc etc. APIs we should in the target region
enforce they don't use omp_null_allocator and the allocators they use
is mentioned in uses_allocators (and implement uses_allocators).
But, right now it is unclear to me how exactly is that supposed to work
with declare target to routines, those aren't nested inside of the target
and so can't know what uses_allocator is used, so they can't make any
allocations?  Or is it just that we can't diagnose this in such routines
and only can diagnose omp_null_allocator isn't used...
> 
> I think this patch ought to be fine for GCC 12, but if not it can wait until
> stage 1 opens. I shall backport this to OG11 shortly.

Anyway, your patch is a step forward, as we don't support uses_allocators,
the non-requires dynamic_allocators way is unusable for any target region
allocations due to that and so requires dynamic_allocators is the only thing
we actually support, but we were rejecting that directive...

Ok and sorry for the delay.

> gcc/c/ChangeLog:
> 
> 	* c-parser.c (c_parser_omp_requires): Don't "sorry" dynamic_allocators.
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.c (cp_parser_omp_requires): Don't "sorry" dynamic_allocators.
> 
> gcc/fortran/ChangeLog:
> 
> 	* openmp.c (gfc_match_omp_requires): Don't "sorry" dynamic_allocators.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gfortran.dg/gomp/requires-8.f90: Reinstate dynamic allocators
> 	requirement.

	Jakub