public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch] OpenMP: Parse align clause in allocate directive in C/C++
@ 2022-12-13 17:44 Tobias Burnus
  2023-01-31 12:09 ` Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Tobias Burnus @ 2022-12-13 17:44 UTC (permalink / raw)
  To: Jakub Jelinek, gcc-patches

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

We have a working parsing support for the 'allocate' directive
(failing immediately with a sorry after parsing).

To be in line with the rest of the allocat(e,or) etc. handling,
it makes sense to take care of 'align' as well, which is this
patch does - it still fails with a 'sorry' after parsing.

OK for mainline?

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: align-c.diff --]
[-- Type: text/x-patch, Size: 10567 bytes --]

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)] */
 
 static void
 c_parser_omp_allocate (location_t loc, c_parser *parser)
 {
+  tree alignment = NULL_TREE;
   tree allocator = NULL_TREE;
   tree nl = c_parser_omp_var_list_parens (parser, OMP_CLAUSE_ALLOCATE, NULL_TREE);
-  if (c_parser_next_token_is (parser, CPP_COMMA)
-      && c_parser_peek_2nd_token (parser)->type == CPP_NAME)
-    c_parser_consume_token (parser);
-  if (c_parser_next_token_is (parser, CPP_NAME))
+  do
     {
+      if (c_parser_next_token_is (parser, CPP_COMMA)
+	  && c_parser_peek_2nd_token (parser)->type == CPP_NAME)
+	c_parser_consume_token (parser);
+      if (!c_parser_next_token_is (parser, CPP_NAME))
+	break;
       matching_parens parens;
       const char *p = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value);
       c_parser_consume_token (parser);
-      if (strcmp ("allocator", p) != 0)
-	error_at (c_parser_peek_token (parser)->location,
-		  "expected %<allocator%>");
-      else if (parens.require_open (parser))
+      location_t expr_loc = c_parser_peek_token (parser)->location;
+      if (strcmp ("align", p) != 0 && strcmp ("allocator", p) != 0)
 	{
-	  location_t expr_loc = c_parser_peek_token (parser)->location;
-	  c_expr expr = c_parser_expr_no_commas (parser, NULL);
-	  expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true);
-	  allocator = expr.value;
-	  allocator = c_fully_fold (allocator, false, NULL);
+	  error_at (c_parser_peek_token (parser)->location,
+		    "expected %<allocator%> or %<align%>");
+	  break;
+	}
+      if (!parens.require_open (parser))
+	break;
+
+      c_expr expr = c_parser_expr_no_commas (parser, NULL);
+      expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true);
+      expr_loc = c_parser_peek_token (parser)->location;
+      if (p[2] == 'i' && alignment)
+	{
+	  error_at (expr_loc, "too many %qs clauses", "align");
+	  break;
+	}
+      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))
+	    {
+	      error_at (expr_loc, "%<align%> clause argument needs to be "
+				  "positive constant power of two integer "
+				  "expression");
+	      alignment = NULL_TREE;
+	    }
+	}
+      else if (allocator)
+	{
+	  error_at (expr_loc, "too many %qs clauses", "allocator");
+	  break;
+	}
+      else
+	{
+	  allocator = c_fully_fold (expr.value, false, NULL);
 	  tree orig_type
 	    = expr.original_type ? expr.original_type : TREE_TYPE (allocator);
 	  orig_type = TYPE_MAIN_VARIANT (orig_type);
@@ -18853,20 +18892,23 @@ c_parser_omp_allocate (location_t loc, c_parser *parser)
 	      || TYPE_NAME (orig_type)
 		 != get_identifier ("omp_allocator_handle_t"))
 	    {
-	      error_at (expr_loc, "%<allocator%> clause allocator expression "
-				"has type %qT rather than "
-				"%<omp_allocator_handle_t%>",
-				TREE_TYPE (allocator));
+	      error_at (expr_loc,
+			"%<allocator%> clause allocator expression has type "
+			"%qT rather than %<omp_allocator_handle_t%>",
+			TREE_TYPE (allocator));
 	      allocator = NULL_TREE;
 	    }
-	  parens.skip_until_found_close (parser);
 	}
-    }
+      parens.skip_until_found_close (parser);
+    } while (true);
   c_parser_skip_to_pragma_eol (parser);
 
-  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;
+      }
 
   sorry_at (loc, "%<#pragma omp allocate%> not yet supported");
 }
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 4798aae1fbb..d284022266f 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -41398,36 +41398,64 @@ static void
 cp_parser_omp_allocate (cp_parser *parser, cp_token *pragma_tok)
 {
   tree allocator = NULL_TREE;
+  tree alignment = NULL_TREE;
   location_t loc = pragma_tok->location;
   tree nl = cp_parser_omp_var_list (parser, OMP_CLAUSE_ALLOCATE, NULL_TREE);
 
-  if (cp_lexer_next_token_is (parser->lexer, CPP_COMMA)
-      && cp_lexer_nth_token_is (parser->lexer, 2, CPP_NAME))
-    cp_lexer_consume_token (parser->lexer);
-
-  if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
+  do
     {
+      if (cp_lexer_next_token_is (parser->lexer, CPP_COMMA)
+	  && cp_lexer_nth_token_is (parser->lexer, 2, CPP_NAME))
+	cp_lexer_consume_token (parser->lexer);
+
+      if (!cp_lexer_next_token_is (parser->lexer, CPP_NAME))
+	break;
       matching_parens parens;
       tree id = cp_lexer_peek_token (parser->lexer)->u.value;
       const char *p = IDENTIFIER_POINTER (id);
       location_t cloc = cp_lexer_peek_token (parser->lexer)->location;
       cp_lexer_consume_token (parser->lexer);
-      if (strcmp (p, "allocator") != 0)
-	error_at (cloc, "expected %<allocator%>");
-      else if (parens.require_open (parser))
+      if (strcmp (p, "allocator") != 0 && strcmp (p, "align") != 0)
 	{
-	  allocator = cp_parser_assignment_expression (parser);
-	  if (allocator == error_mark_node)
-	    allocator = NULL_TREE;
-	  parens.require_close (parser);
+	  error_at (cloc, "expected %<allocator%> or %<align%>");
+	  break;
 	}
-    }
+      if (!parens.require_open (parser))
+	break;
+      tree expr = cp_parser_assignment_expression (parser);
+      if (p[2] == 'i' && alignment)
+	{
+	  error_at (cloc, "too many %qs clauses", "align");
+	  break;
+	}
+      else if (p[2] == 'i')
+	{
+	  if (expr != error_mark_node)
+	    alignment = expr;
+	}
+      else if (allocator)
+	{
+	  error_at (cloc, "too many %qs clauses", "allocator");
+	  break;
+	}
+      else
+	{
+	  if (expr != error_mark_node)
+	    allocator = expr;
+	}
+      parens.require_close (parser);
+    } while (true);
   cp_parser_require_pragma_eol (parser, pragma_tok);
 
-  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;
+      }
 
+  /* align/allocator needs same check as the modifiers to the
+     'allocator' clause in semantics.cc's finish_omp_clauses.  */
   sorry_at (loc, "%<#pragma omp allocate%> not yet supported");
 }
 
diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-5.c b/gcc/testsuite/c-c++-common/gomp/allocate-5.c
index 34dcb48c3d7..997e7f06896 100644
--- a/gcc/testsuite/c-c++-common/gomp/allocate-5.c
+++ b/gcc/testsuite/c-c++-common/gomp/allocate-5.c
@@ -39,3 +39,39 @@ bar ()
 #pragma omp allocate(a) allocator(b)  /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */
   /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
 }
+
+
+void
+align_test ()
+{
+  int i;
+  #pragma omp allocate(i) allocator(omp_default_mem_alloc), align(32)
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+  #pragma omp allocate(i) align ( 32 ),allocator(omp_default_mem_alloc)
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+  #pragma omp allocate(i),allocator(omp_default_mem_alloc) align(32)
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+  #pragma omp allocate(i) align ( 32 ) allocator(omp_default_mem_alloc)
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+
+  #pragma omp allocate(i) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc)
+  /* { dg-error "too many 'allocator' clauses" "" { target *-*-* } .-1 } */
+  /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */
+  #pragma omp allocate(i) align ( 32 ), align(32) allocator(omp_default_mem_alloc)
+  /* { dg-error "too many 'align' clauses" "" { target *-*-* } .-1 } */
+  /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */
+}
+
+void
+align_test2 ()
+{
+  int i;
+  #pragma omp allocate(i) align (32.0)  /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" "todo: cp/semantics.c" { xfail c++ } } */
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+  #pragma omp allocate(i) align ( 31 )  /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" "todo: cp/semantics.c" { xfail c++ } } */
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+  #pragma omp allocate(i) align ( -32 )  /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" "todo: cp/semantics.c" { xfail c++ } } */
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+}

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

* Re: [Patch] OpenMP: Parse align clause in allocate directive in C/C++
  2022-12-13 17:44 [Patch] OpenMP: Parse align clause in allocate directive in C/C++ Tobias Burnus
@ 2023-01-31 12:09 ` Jakub Jelinek
  2023-02-09 10:16   ` Tobias Burnus
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2023-01-31 12:09 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: gcc-patches

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


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

* Re: [Patch] OpenMP: Parse align clause in allocate directive in C/C++
  2023-01-31 12:09 ` Jakub Jelinek
@ 2023-02-09 10:16   ` Tobias Burnus
  2023-02-09 10:25     ` Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Tobias Burnus @ 2023-02-09 10:16 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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

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

[-- Attachment #2: align-c-v2.diff --]
[-- Type: text/x-patch, Size: 11930 bytes --]

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                             | 97 +++++++++++++++++++++++-----
 gcc/testsuite/c-c++-common/gomp/allocate-5.c | 36 +++++++++++
 3 files changed, 181 insertions(+), 40 deletions(-)

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 69230002bc8..43427886ad4 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -18814,32 +18814,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 (constant-expression)]  */
 
 static void
 c_parser_omp_allocate (location_t loc, c_parser *parser)
 {
+  tree alignment = NULL_TREE;
   tree allocator = NULL_TREE;
   tree nl = c_parser_omp_var_list_parens (parser, OMP_CLAUSE_ALLOCATE, NULL_TREE);
-  if (c_parser_next_token_is (parser, CPP_COMMA)
-      && c_parser_peek_2nd_token (parser)->type == CPP_NAME)
-    c_parser_consume_token (parser);
-  if (c_parser_next_token_is (parser, CPP_NAME))
+  do
     {
+      if (c_parser_next_token_is (parser, CPP_COMMA)
+	  && c_parser_peek_2nd_token (parser)->type == CPP_NAME)
+	c_parser_consume_token (parser);
+      if (!c_parser_next_token_is (parser, CPP_NAME))
+	break;
       matching_parens parens;
       const char *p = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value);
       c_parser_consume_token (parser);
-      if (strcmp ("allocator", p) != 0)
-	error_at (c_parser_peek_token (parser)->location,
-		  "expected %<allocator%>");
-      else if (parens.require_open (parser))
+      location_t expr_loc = c_parser_peek_token (parser)->location;
+      if (strcmp ("align", p) != 0 && strcmp ("allocator", p) != 0)
 	{
-	  location_t expr_loc = c_parser_peek_token (parser)->location;
-	  c_expr expr = c_parser_expr_no_commas (parser, NULL);
-	  expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true);
-	  allocator = expr.value;
-	  allocator = c_fully_fold (allocator, false, NULL);
+	  error_at (c_parser_peek_token (parser)->location,
+		    "expected %<allocator%> or %<align%>");
+	  break;
+	}
+      if (!parens.require_open (parser))
+	break;
+
+      c_expr expr = c_parser_expr_no_commas (parser, NULL);
+      expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true);
+      expr_loc = c_parser_peek_token (parser)->location;
+      if (p[2] == 'i' && alignment)
+	{
+	  error_at (expr_loc, "too many %qs clauses", "align");
+	  break;
+	}
+      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))
+	    {
+	      error_at (expr_loc, "%<align%> clause argument needs to be "
+				  "positive constant power of two integer "
+				  "expression");
+	      alignment = NULL_TREE;
+	    }
+	}
+      else if (allocator)
+	{
+	  error_at (expr_loc, "too many %qs clauses", "allocator");
+	  break;
+	}
+      else
+	{
+	  allocator = c_fully_fold (expr.value, false, NULL);
 	  tree orig_type
 	    = expr.original_type ? expr.original_type : TREE_TYPE (allocator);
 	  orig_type = TYPE_MAIN_VARIANT (orig_type);
@@ -18848,20 +18887,23 @@ c_parser_omp_allocate (location_t loc, c_parser *parser)
 	      || TYPE_NAME (orig_type)
 		 != get_identifier ("omp_allocator_handle_t"))
 	    {
-	      error_at (expr_loc, "%<allocator%> clause allocator expression "
-				"has type %qT rather than "
-				"%<omp_allocator_handle_t%>",
-				TREE_TYPE (allocator));
+	      error_at (expr_loc,
+			"%<allocator%> clause allocator expression has type "
+			"%qT rather than %<omp_allocator_handle_t%>",
+			TREE_TYPE (allocator));
 	      allocator = NULL_TREE;
 	    }
-	  parens.skip_until_found_close (parser);
 	}
-    }
+      parens.skip_until_found_close (parser);
+    } while (true);
   c_parser_skip_to_pragma_eol (parser);
 
-  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;
+      }
 
   sorry_at (loc, "%<#pragma omp allocate%> not yet supported");
 }
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 4cdc1cd472f..a89ab284a38 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -41483,43 +41483,106 @@ cp_parser_omp_structured_block (cp_parser *parser, bool *if_p)
   return finish_omp_structured_block (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 (constant-expression)]  */
 
 static void
 cp_parser_omp_allocate (cp_parser *parser, cp_token *pragma_tok)
 {
   tree allocator = NULL_TREE;
+  tree alignment = NULL_TREE;
   location_t loc = pragma_tok->location;
   tree nl = cp_parser_omp_var_list (parser, OMP_CLAUSE_ALLOCATE, NULL_TREE);
 
-  if (cp_lexer_next_token_is (parser->lexer, CPP_COMMA)
-      && cp_lexer_nth_token_is (parser->lexer, 2, CPP_NAME))
-    cp_lexer_consume_token (parser->lexer);
-
-  if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
+  do
     {
+      if (cp_lexer_next_token_is (parser->lexer, CPP_COMMA)
+	  && cp_lexer_nth_token_is (parser->lexer, 2, CPP_NAME))
+	cp_lexer_consume_token (parser->lexer);
+
+      if (!cp_lexer_next_token_is (parser->lexer, CPP_NAME))
+	break;
       matching_parens parens;
       tree id = cp_lexer_peek_token (parser->lexer)->u.value;
       const char *p = IDENTIFIER_POINTER (id);
       location_t cloc = cp_lexer_peek_token (parser->lexer)->location;
       cp_lexer_consume_token (parser->lexer);
-      if (strcmp (p, "allocator") != 0)
-	error_at (cloc, "expected %<allocator%>");
-      else if (parens.require_open (parser))
+      if (strcmp (p, "allocator") != 0 && strcmp (p, "align") != 0)
 	{
-	  allocator = cp_parser_assignment_expression (parser);
-	  if (allocator == error_mark_node)
-	    allocator = NULL_TREE;
-	  parens.require_close (parser);
+	  error_at (cloc, "expected %<allocator%> or %<align%>");
+	  break;
 	}
-    }
+      if (!parens.require_open (parser))
+	break;
+      tree expr = cp_parser_assignment_expression (parser);
+      if (p[2] == 'i' && alignment)
+	{
+	  error_at (cloc, "too many %qs clauses", "align");
+	  break;
+	}
+      else if (p[2] == 'i')
+	{
+	  if (expr != error_mark_node)
+	    alignment = expr;
+	  /* FIXME: Remove when adding check to semantic.cc; cf FIXME below.  */
+	  if (alignment
+	      && !type_dependent_expression_p (alignment)
+	      && !INTEGRAL_TYPE_P (TREE_TYPE (alignment)))
+	    {
+              error_at (cloc, "%<align%> clause argument needs to be "
+			      "positive constant power of two integer "
+			      "expression");
+	      alignment = NULL_TREE;
+	    }
+	  else if (alignment)
+	    {
+	      alignment = mark_rvalue_use (alignment);
+	      if (!processing_template_decl)
+		{
+		  alignment = maybe_constant_value (alignment);
+		  if (TREE_CODE (alignment) != INTEGER_CST
+		      || !tree_fits_uhwi_p (alignment)
+		      || !integer_pow2p (alignment))
+		    {
+		      error_at (cloc, "%<align%> clause argument needs to be "
+				      "positive constant power of two integer "
+				      "expression");
+		      alignment = NULL_TREE;
+		    }
+		}
+	    }
+	}
+      else if (allocator)
+	{
+	  error_at (cloc, "too many %qs clauses", "allocator");
+	  break;
+	}
+      else
+	{
+	  if (expr != error_mark_node)
+	    allocator = expr;
+	}
+      parens.require_close (parser);
+    } while (true);
   cp_parser_require_pragma_eol (parser, pragma_tok);
 
-  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;
+      }
 
+  /* FIXME: When implementing properly, delete the align/allocate expr error
+     check above and add one in semantics.cc (to properly handle templates).
+     Base this on the allocator/align modifiers check for the 'allocate' clause
+     in semantics.cc's finish_omp_clauses.  */
   sorry_at (loc, "%<#pragma omp allocate%> not yet supported");
 }
 
diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-5.c b/gcc/testsuite/c-c++-common/gomp/allocate-5.c
index 34dcb48c3d7..8a9181205f7 100644
--- a/gcc/testsuite/c-c++-common/gomp/allocate-5.c
+++ b/gcc/testsuite/c-c++-common/gomp/allocate-5.c
@@ -39,3 +39,39 @@ bar ()
 #pragma omp allocate(a) allocator(b)  /* { dg-error "'allocator' clause allocator expression has type 'int' rather than 'omp_allocator_handle_t'" "todo: cp/semantics.c" { xfail c++ } } */
   /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
 }
+
+
+void
+align_test ()
+{
+  int i;
+  #pragma omp allocate(i) allocator(omp_default_mem_alloc), align(32)
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+  #pragma omp allocate(i) align ( 32 ),allocator(omp_default_mem_alloc)
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+  #pragma omp allocate(i),allocator(omp_default_mem_alloc) align(32)
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+  #pragma omp allocate(i) align ( 32 ) allocator(omp_default_mem_alloc)
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+
+  #pragma omp allocate(i) allocator ( omp_high_bw_mem_alloc ), align ( 32 ) allocator(omp_default_mem_alloc)
+  /* { dg-error "too many 'allocator' clauses" "" { target *-*-* } .-1 } */
+  /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */
+  #pragma omp allocate(i) align ( 32 ), align(32) allocator(omp_default_mem_alloc)
+  /* { dg-error "too many 'align' clauses" "" { target *-*-* } .-1 } */
+  /* { dg-error "expected end of line before '\\)' token" "" { target *-*-* } .-2 } */
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-3 } */
+}
+
+void
+align_test2 ()
+{
+  int i;
+  #pragma omp allocate(i) align (32.0)  /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+  #pragma omp allocate(i) align ( 31 )  /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+  #pragma omp allocate(i) align ( -32 )  /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */
+  /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target *-*-* } .-1 } */
+}

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

* Re: [Patch] OpenMP: Parse align clause in allocate directive in C/C++
  2023-02-09 10:16   ` Tobias Burnus
@ 2023-02-09 10:25     ` Jakub Jelinek
  0 siblings, 0 replies; 4+ messages in thread
From: Jakub Jelinek @ 2023-02-09 10:25 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: gcc-patches

On Thu, Feb 09, 2023 at 11:16:39AM +0100, Tobias Burnus wrote:
> Any further comments before I commit it?
> 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.

The " and check for restrictions" part now applies also to C++...

> +	  if (expr != error_mark_node)
> +	    alignment = expr;
> +	  /* FIXME: Remove when adding check to semantic.cc; cf FIXME below.  */

s/semantic.cc/semantics.cc/

Ok with those nits fixed.

	Jakub


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

end of thread, other threads:[~2023-02-09 10:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-13 17:44 [Patch] OpenMP: Parse align clause in allocate directive in C/C++ Tobias Burnus
2023-01-31 12:09 ` Jakub Jelinek
2023-02-09 10:16   ` Tobias Burnus
2023-02-09 10:25     ` Jakub Jelinek

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