Updated patch included. Changes:

* Removed xfail for C++

* For C, I updated the comment as suggested.

* For C++: I updated/extended the FIXME comment and added the 'align'
check (the simple version as first suggested; I did not went for the one
which supports some templates.)

Any further comments before I commit it?

Tobias

On 31.01.23 13:09, Jakub Jelinek wrote:
> 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.
-----------------
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