Re: [Patch] OpenMP: Parse align clause in allocate directive in C/C++

[Jakub Jelinek <jakub@redhat.com>]

On Tue, Dec 13, 2022 at 06:44:27PM +0100, Tobias Burnus wrote:
> OpenMP: Parse align clause in allocate directive in C/C++
> 
> gcc/c/ChangeLog:
> 
> 	* c-parser.cc (c_parser_omp_allocate): Parse align
> 	clause and check for restrictions.
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_omp_allocate): Parse align
> 	clause.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* c-c++-common/gomp/allocate-5.c: Extend for align clause.
> 
>  gcc/c/c-parser.cc                            | 88 ++++++++++++++++++++--------
>  gcc/cp/parser.cc                             | 58 +++++++++++++-----
>  gcc/testsuite/c-c++-common/gomp/allocate-5.c | 36 ++++++++++++
>  3 files changed, 144 insertions(+), 38 deletions(-)
> 
> diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
> index 1bbb39f9b08..62c302748dd 100644
> --- a/gcc/c/c-parser.cc
> +++ b/gcc/c/c-parser.cc
> @@ -18819,32 +18819,71 @@ c_parser_oacc_wait (location_t loc, c_parser *parser, char *p_name)
>    return stmt;
>  }
>  
> -/* OpenMP 5.0:
> -   # pragma omp allocate (list)  [allocator(allocator)]  */
> +/* OpenMP 5.x:
> +   # pragma omp allocate (list)  clauses
> +
> +   OpenMP 5.0 clause:
> +   allocator (omp_allocator_handle_t expression)
> +
> +   OpenMP 5.1 additional clause:
> +   align (int expression)] */

integer-expression or constant-expression or just expression.
Also, 2 spaces before */ rather than just one.

> +      else if (p[2] == 'i')
> +	{
> +	  alignment = c_fully_fold (expr.value, false, NULL);
> +	  if (TREE_CODE (alignment) != INTEGER_CST
> +	      || !INTEGRAL_TYPE_P (TREE_TYPE (alignment))
> +	      || tree_int_cst_sgn (alignment) != 1
> +	      || !integer_pow2p (alignment))

Note, the reason why we diagnose this in c-parser.cc and for C++
in semantics.cc rather than in parser.cc are templates, it would be
wasteful to handle it in two spots (parser.cc and during instantiation).

> -  if (allocator)
> +  if (allocator || alignment)
>      for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c))
> -      OMP_CLAUSE_ALLOCATE_ALLOCATOR (c) = allocator;
> +      {
> +	OMP_CLAUSE_ALLOCATE_ALLOCATOR (c) = allocator;
> +	OMP_CLAUSE_ALLOCATE_ALIGN (c) = alignment;
> +      }

So, if you want for now until we actually support the directive
properly diagnose it here in the parser (otherwise there is nothing
to stick it on for later), then it would need either what semantics.cc
currently uses:
              if (!type_dependent_expression_p (align)
                  && !INTEGRAL_TYPE_P (TREE_TYPE (align)))
                {
                  error_at (OMP_CLAUSE_LOCATION (c),
                            "%<allocate%> clause %<align%> modifier "
                            "argument needs to be positive constant "
                            "power of two integer expression");
                  remove = true;
                }
              else
                {
                  align = mark_rvalue_use (align);
                  if (!processing_template_decl)
                    {
                      align = maybe_constant_value (align);
                      if (TREE_CODE (align) != INTEGER_CST
                          || !tree_fits_uhwi_p (align)
                          || !integer_pow2p (align))
                        {
                          error_at (OMP_CLAUSE_LOCATION (c),
                                    "%<allocate%> clause %<align%> modifier "
                                    "argument needs to be positive constant "
                                    "power of two integer expression");
                          remove = true;
                        }
                    }
                }
with adjusted diagnostics, or perhaps instead of the
!processing_template_decl guarding do
fold_non_dependent_expr (align, !tf_none)
instead of maybe_constant_value and just for
processing_template_decl && TREE_CODE (align) != INTEGER_CST
do nothing instead of the other tests and diagnostics.
With the !processing_template_decl guarding it wouldn't diagnose
ever non-power of two or non-constant operand in templates,
with fold_non_dependent_expr etc. it would as long as they are
not dependent.

Otherwise LGTM, with or without the above changes.

	Jakub