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
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 } */
+}
