* [Patch] OpenMP: Add C++ support for 'omp allocate' with stack variables
@ 2023-10-20 16:49 Tobias Burnus
2023-11-07 8:14 ` *ping* / " Tobias Burnus
2023-12-08 11:28 ` Jakub Jelinek
0 siblings, 2 replies; 3+ messages in thread
From: Tobias Burnus @ 2023-10-20 16:49 UTC (permalink / raw)
To: gcc-patches, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 597 bytes --]
This patch adds C++ support for OpenMP's 'omp allocate' for
stack/automatic arrays.
Comments and suggestions? — I bet there are given my little knowledge
about the C++ FE...
Tobias
PS: I think I should write some additional C++-specific code, I bet some
corner cases are missed. The question is only which ones.
-----------------
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: omp-allocate-c++.diff --]
[-- Type: text/x-patch, Size: 38086 bytes --]
OpenMP: Add C++ support for 'omp allocate' with stack variables
gcc/c/ChangeLog:
* c-parser.cc (c_parser_omp_allocate): Change error
wording.
gcc/cp/ChangeLog:
* cp-tree.h (finish_omp_allocate): New prototype.
* parser.cc (struct cp_omp_loc_tree,
cp_check_omp_allocate_allocator_r): New.
(cp_parser_omp_allocate): Call it; remove sorry,
improve checks, call finish_omp_allocate.
* pt.cc (tsubst_stmt): Call finish_omp_allocate.
* semantics.cc (finish_omp_allocate): New.
libgomp/ChangeLog:
* libgomp.texi (OpenMP Impl. Status): Document that 'omp allocate'
is now supported for C++ stack/automatic variables.
* testsuite/libgomp.c-c++-common/allocate-4.c: Renamed from ...
testsuite/libgomp.c/allocate-4.c: ... this.
* testsuite/libgomp.c-c++-common/allocate-5.c: Renamed from ...
testsuite/libgomp.c/allocate-5.c: ... this.
* testsuite/libgomp.c-c++-common/allocate-6.c: Renamed from ...
testsuite/libgomp.c/allocate-6.c: ... this.
* testsuite/libgomp.c++/allocate-2.C: New test.
gcc/testsuite/ChangeLog:
* c-c++-common/gomp/allocate-5.c: Remove C++ 'sorry'; minor updates.
* c-c++-common/gomp/allocate-9.c: Likewise.
* c-c++-common/gomp/allocate-17.c: Likewise.
* c-c++-common/gomp/directive-1.c: Likewise.
* g++.dg/gomp/allocate-5.C: New test.
gcc/c/c-parser.cc | 2 +-
gcc/cp/cp-tree.h | 4 +
gcc/cp/parser.cc | 149 +++++++---
gcc/cp/pt.cc | 4 +
gcc/cp/semantics.cc | 80 +++++
gcc/testsuite/c-c++-common/gomp/allocate-17.c | 2 +-
gcc/testsuite/c-c++-common/gomp/allocate-5.c | 24 +-
gcc/testsuite/c-c++-common/gomp/allocate-9.c | 59 ++--
gcc/testsuite/g++.dg/gomp/allocate-5.C | 14 +
libgomp/libgomp.texi | 4 +-
libgomp/testsuite/libgomp.c++/allocate-2.C | 329 +++++++++++++++++++++
.../allocate-4.c | 3 -
.../allocate-5.c | 3 -
.../allocate-6.c | 3 -
14 files changed, 567 insertions(+), 113 deletions(-)
diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 0d468b86bd8..49befd6ffc4 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -19501,7 +19501,7 @@ c_parser_omp_allocate (c_parser *parser)
!= get_identifier ("omp_allocator_handle_t"))
{
error_at (expr_loc,
- "%<allocator%> clause allocator expression has type "
+ "%<allocator%> clause expression has type "
"%qT rather than %<omp_allocator_handle_t%>",
TREE_TYPE (allocator));
allocator = NULL_TREE;
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 30fe716b109..d3a85f39e3e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7801,6 +7801,10 @@ extern tree finish_omp_for (location_t, enum tree_code,
tree, tree, tree, tree, tree,
tree, tree, vec<tree> *, tree);
extern tree finish_omp_for_block (tree, tree);
+extern void finish_omp_allocate (bool, location_t, tree,
+ tree = NULL_TREE,
+ tsubst_flags_t = tf_warning_or_error,
+ tree = NULL_TREE);
extern void finish_omp_atomic (location_t, enum tree_code,
enum tree_code, tree, tree,
tree, tree, tree, tree, tree,
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 5483121f51c..b163618292e 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -41864,6 +41864,67 @@ cp_parser_omp_structured_block (cp_parser *parser, bool *if_p)
return finish_omp_structured_block (stmt);
}
+struct cp_omp_loc_tree
+{
+ location_t loc;
+ tree var;
+};
+
+/* Check whether the expression used in the allocator clause is declared or
+ modified between the variable declaration and its allocate directive. */
+static tree
+cp_check_omp_allocate_allocator_r (tree *tp, int *, void *data)
+{
+ tree var = ((struct cp_omp_loc_tree *) data)->var;
+ location_t loc = ((struct cp_omp_loc_tree *) data)->loc;
+ tree v = NULL_TREE;
+ if (TREE_CODE (*tp) == VAR_DECL)
+ for (v = current_binding_level->names; v; v = TREE_CHAIN (v))
+ if (v == var)
+ break;
+ if (v != NULL_TREE)
+ {
+ if (linemap_location_before_p (line_table, DECL_SOURCE_LOCATION (var),
+ DECL_SOURCE_LOCATION (*tp)))
+ {
+ error_at (loc, "variable %qD used in the %<allocator%> clause must "
+ "be declared before %qD", *tp, var);
+ inform (DECL_SOURCE_LOCATION (*tp), "declared here");
+ inform (DECL_SOURCE_LOCATION (var),
+ "to be allocated variable declared here");
+ return *tp;
+ }
+ else
+ {
+ gcc_assert (cur_stmt_list
+ && TREE_CODE (cur_stmt_list) == STATEMENT_LIST);
+
+ tree_stmt_iterator l = tsi_last (cur_stmt_list);
+ while (!tsi_end_p (l))
+ {
+ if (linemap_location_before_p (line_table, EXPR_LOCATION (*l),
+ DECL_SOURCE_LOCATION (var)))
+ break;
+ if (TREE_CODE (*l) == MODIFY_EXPR
+ && TREE_OPERAND (*l, 0) == *tp)
+ {
+ error_at (loc,
+ "variable %qD used in the %<allocator%> clause "
+ "must not be modified between declaration of %qD "
+ "and its %<allocate%> directive", *tp, var);
+ inform (EXPR_LOCATION (*l), "modified here");
+ inform (DECL_SOURCE_LOCATION (var),
+ "to be allocated variable declared here");
+ return *tp;
+ }
+ --l;
+ }
+ }
+ }
+ return NULL_TREE;
+}
+
+
/* OpenMP 5.x:
# pragma omp allocate (list) clauses
@@ -41878,7 +41939,6 @@ 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);
do
@@ -41908,63 +41968,56 @@ cp_parser_omp_allocate (cp_parser *parser, cp_token *pragma_tok)
break;
}
else if (p[2] == 'i')
- {
- if (expr != error_mark_node)
- alignment = expr;
- /* FIXME: Remove when adding check to semantics.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;
- }
- }
- }
- }
+ alignment = expr;
else if (allocator)
{
error_at (cloc, "too many %qs clauses", "allocator");
break;
}
else
- {
- if (expr != error_mark_node)
- allocator = expr;
- }
+ allocator = expr;
parens.require_close (parser);
} while (true);
cp_parser_require_pragma_eol (parser, pragma_tok);
- if (allocator || alignment)
- for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c))
- {
- 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");
+ for (tree c = nl; c != NULL_TREE; c = OMP_CLAUSE_CHAIN (c))
+ {
+ tree var = OMP_CLAUSE_DECL (c);
+ if (lookup_attribute ("omp allocate", DECL_ATTRIBUTES (var)))
+ {
+ error_at (OMP_CLAUSE_LOCATION (nl),
+ "%qD already appeared as list item in an "
+ "%<allocate%> directive", var);
+ continue;
+ }
+ if (TREE_CODE (var) == PARM_DECL)
+ {
+ error_at (OMP_CLAUSE_LOCATION (nl),
+ "function parameter %qD may not appear as list item in an "
+ "%<allocate%> directive", var);
+ continue;
+ }
+ tree v;
+ for (v = current_binding_level->names; v; v = TREE_CHAIN (v))
+ if (v == var)
+ break;
+ if (v == NULL_TREE)
+ {
+ error_at (OMP_CLAUSE_LOCATION (nl),
+ "%<allocate%> directive must be in the same scope as %qD",
+ var);
+ inform (DECL_SOURCE_LOCATION (var), "declared here");
+ continue;
+ }
+ struct cp_omp_loc_tree data
+ = {EXPR_LOC_OR_LOC (allocator, OMP_CLAUSE_LOCATION (nl)), var};
+ walk_tree (&allocator, cp_check_omp_allocate_allocator_r, &data, NULL);
+ DECL_ATTRIBUTES (var) = tree_cons (get_identifier ("omp allocate"),
+ build_tree_list (allocator, alignment),
+ DECL_ATTRIBUTES (var));
+ if (!processing_template_decl)
+ finish_omp_allocate (true, OMP_CLAUSE_LOCATION (nl), var);
+ }
}
/* OpenMP 2.5:
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 210c6cb9e4d..10603f4c39f 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -18379,6 +18379,10 @@ tsubst_stmt (tree t, tree args, tsubst_flags_t complain, tree in_decl)
cp_finish_decl (decl, init, const_init, asmspec_tree, 0,
decomp);
+
+ if (flag_openmp && VAR_P (decl))
+ finish_omp_allocate (false, DECL_SOURCE_LOCATION (decl),
+ decl, args, complain, in_decl);
if (ndecl != error_mark_node)
cp_finish_decomp (ndecl, decomp);
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index dc3c11461fb..30c70c0b13e 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -10987,6 +10987,86 @@ finish_omp_for_block (tree bind, tree omp_for)
return bind;
}
+void
+finish_omp_allocate (bool in_parsing, location_t loc, tree decl, tree args,
+ tsubst_flags_t complain, tree in_decl)
+{
+ location_t loc2;
+ tree attr = lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl));
+ if (attr == NULL_TREE)
+ return;
+
+ tree allocator = TREE_PURPOSE (TREE_VALUE (attr));
+ tree alignment = TREE_VALUE (TREE_VALUE (attr));
+
+ if (alignment == error_mark_node)
+ TREE_VALUE (TREE_VALUE (attr)) = NULL_TREE;
+ else if (alignment)
+ {
+ location_t loc2 = EXPR_LOCATION (alignment);
+ if (!in_parsing)
+ alignment = tsubst_expr (alignment, args, complain, in_decl);
+ alignment = fold_non_dependent_expr (alignment);
+
+ if (TREE_CODE (alignment) != INTEGER_CST
+ || !INTEGRAL_TYPE_P (TREE_TYPE (alignment))
+ || tree_int_cst_sgn (alignment) != 1
+ || !integer_pow2p (alignment))
+ {
+ error_at (loc2, "%<align%> clause argument needs to be positive "
+ "constant power of two integer expression");
+ TREE_VALUE (TREE_VALUE (attr)) = NULL_TREE;
+ }
+ else
+ TREE_VALUE (TREE_VALUE (attr)) = alignment;
+ }
+ loc2 = allocator ? EXPR_LOCATION (allocator) : UNKNOWN_LOCATION;
+ if (allocator == error_mark_node)
+ {
+ allocator = TREE_PURPOSE (TREE_VALUE (attr)) = NULL_TREE;
+ return;
+ }
+ else
+ {
+ if (!in_parsing)
+ allocator = tsubst_expr (allocator, args, complain, in_decl);
+ allocator = fold_non_dependent_expr (allocator);
+ }
+
+ if (allocator)
+ {
+ tree orig_type = TYPE_MAIN_VARIANT (TREE_TYPE (allocator));
+ if (!INTEGRAL_TYPE_P (TREE_TYPE (allocator))
+ || TREE_CODE (orig_type) != ENUMERAL_TYPE
+ || TYPE_NAME (orig_type) == NULL_TREE
+ || (DECL_NAME (TYPE_NAME (orig_type))
+ != get_identifier ("omp_allocator_handle_t")))
+ {
+ error_at (loc2, "%<allocator%> clause expression has type "
+ "%qT rather than %<omp_allocator_handle_t%>",
+ TREE_TYPE (allocator));
+ allocator = TREE_PURPOSE (TREE_VALUE (attr)) = NULL_TREE;
+ }
+ else
+ TREE_PURPOSE (TREE_VALUE (attr)) = fold_non_dependent_expr (allocator);
+ }
+ if (TREE_STATIC (decl))
+ {
+ if (allocator == NULL_TREE)
+ error_at (loc, "%<allocator%> clause required for "
+ "static variable %qD", decl);
+ else if (allocator
+ && (wi::to_widest (allocator) < 1
+ || wi::to_widest (allocator) > 8))
+ /* 8 = largest predefined memory allocator. */
+ error_at (loc2, "%<allocator%> clause requires a predefined allocator "
+ "as %qD is static", decl);
+ else
+ sorry_at (loc, "%<#pragma omp allocate%> for static variables like %qD "
+ "not yet supported", decl);
+ }
+}
+
void
finish_omp_atomic (location_t loc, enum tree_code code, enum tree_code opcode,
tree lhs, tree rhs, tree v, tree lhs1, tree rhs1, tree r,
diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-17.c b/gcc/testsuite/c-c++-common/gomp/allocate-17.c
index f75af0c2d93..2a896cc334d 100644
--- a/gcc/testsuite/c-c++-common/gomp/allocate-17.c
+++ b/gcc/testsuite/c-c++-common/gomp/allocate-17.c
@@ -20,7 +20,7 @@ one ()
#pragma omp target map(tofrom: result) firstprivate(n)
{
int var = 5; //, var2[n];
- #pragma omp allocate(var) align(128) allocator(omp_low_lat_mem_alloc) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */
+ #pragma omp allocate(var) align(128) allocator(omp_low_lat_mem_alloc)
var = 7;
}
diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-5.c b/gcc/testsuite/c-c++-common/gomp/allocate-5.c
index 2ca4786264f..b9eed152613 100644
--- a/gcc/testsuite/c-c++-common/gomp/allocate-5.c
+++ b/gcc/testsuite/c-c++-common/gomp/allocate-5.c
@@ -21,11 +21,10 @@ foo ()
omp_allocator_handle_t my_allocator = omp_default_mem_alloc;
int a, b;
static int c;
-#pragma omp allocate (a) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */
-#pragma omp allocate (b) allocator(my_allocator) /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } } */
+#pragma omp allocate (a)
+#pragma omp allocate (b) allocator(my_allocator)
#pragma omp allocate(c) align(32)
- /* { dg-message "'allocator' clause required for static variable 'c'" "" { target c } .-1 } */
- /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */
+ /* { dg-message "'allocator' clause required for static variable 'c'" "" { target *-*-* } .-1 } */
}
void
@@ -34,14 +33,12 @@ bar ()
int a, a2, b;
omp_allocator_handle_t my_allocator;
#pragma omp allocate /* { dg-error "expected '\\(' before end of line" } */
- /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
#pragma omp allocate allocator(my_allocator) /* { dg-error "expected '\\(' before 'allocator'" } */
- /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
#pragma omp allocate(a) foo(my_allocator) /* { dg-error "expected 'allocator'" } */
/* { dg-error "expected end of line before '\\(' token" "" { target *-*-* } .-1 } */
- /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */
-#pragma omp allocate(a2) 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 c++ } .-1 } */
+#pragma omp allocate(a2) allocator(b) /* { dg-error "'allocator' clause expression has type 'int' rather than 'omp_allocator_handle_t'" } */
+ /* The following error is correct albeit slightly surprising: */
+ /* { dg-error "variable 'b' used in the 'allocator' clause must be declared before 'a2'" "" { target c++ } .-2 } */
}
@@ -50,22 +47,16 @@ align_test ()
{
int i1,i2,i3,i4,i5,i6;
#pragma omp allocate(i1) allocator(omp_default_mem_alloc), align(32)
- /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
#pragma omp allocate(i2) align ( 32 ),allocator(omp_default_mem_alloc)
- /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
#pragma omp allocate(i3),allocator(omp_default_mem_alloc) align(32)
- /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
#pragma omp allocate(i4) align ( 32 ) allocator(omp_default_mem_alloc)
- /* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
#pragma omp allocate(i5) 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 c++ } .-3 } */
#pragma omp allocate(i6) 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 c++ } .-3 } */
}
void
@@ -73,9 +64,6 @@ align_test2 ()
{
int i, i2,i3;
#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 c++ } .-1 } */
#pragma omp allocate(i2) 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 c++ } .-1 } */
#pragma omp allocate(i3) 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 c++ } .-1 } */
}
diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-9.c b/gcc/testsuite/c-c++-common/gomp/allocate-9.c
index 31382748be6..aaa52b359f8 100644
--- a/gcc/testsuite/c-c++-common/gomp/allocate-9.c
+++ b/gcc/testsuite/c-c++-common/gomp/allocate-9.c
@@ -17,7 +17,11 @@ typedef enum omp_allocator_handle_t
} omp_allocator_handle_t;
-static int A[5] = {1,2,3,4,5};
+static int A1[5] = {1,2,3,4,5};
+static int A2[5] = {1,2,3,4,5};
+static int A3[5] = {1,2,3,4,5};
+static int A4[5] = {1,2,3,4,5};
+static int A5[5] = {1,2,3,4,5};
int B, C, D;
/* If the following fails because of added predefined allocators, please update
@@ -27,72 +31,60 @@ int B, C, D;
- libgomp/libgomp.texi (document the new values - multiple locations)
+ ensure that the memory-spaces are also up to date. */
-#pragma omp allocate(A) align(32) allocator((omp_allocator_handle_t) 9) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
+#pragma omp allocate(A1) align(32) allocator((omp_allocator_handle_t) 9) /* { dg-error "'allocator' clause requires a predefined allocator as 'A1' is static" } */
// typo in allocator name:
-#pragma omp allocate(A) allocator(omp_low_latency_mem_alloc)
+#pragma omp allocate(A2) allocator(omp_low_latency_mem_alloc)
/* { dg-error "'omp_low_latency_mem_alloc' undeclared here \\(not in a function\\); did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c } .-1 } */
/* { dg-error "'omp_low_latency_mem_alloc' was not declared in this scope; did you mean 'omp_low_lat_mem_alloc'\\?" "" { target c++ } .-2 } */
-/* { dg-error "'allocator' clause required for static variable 'A'" "" { target c } .-3 } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-4 } */
+/* { dg-error "'allocator' clause required for static variable 'A2'" "" { target c } .-3 } */
/* align be const multiple of 2 */
-#pragma omp allocate(A) align(31) allocator(omp_default_mem_alloc) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'A' not yet supported" "" { target c } .-1 } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */
+#pragma omp allocate(A3) align(31) allocator(omp_default_mem_alloc) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */
+/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'A3' not yet supported" "" { target *-*-* } .-1 } */
/* allocator missing (required as A is static) */
-#pragma omp allocate(A) align(32) /* { dg-error "'allocator' clause required for static variable 'A'" "" { xfail c++ } } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
+#pragma omp allocate(A4) align(32) /* { dg-error "'allocator' clause required for static variable 'A4'" } */
/* "expression in the clause must be a constant expression that evaluates to one of the
predefined memory allocator values -> omp_low_lat_mem_alloc" */
#pragma omp allocate(B) allocator((omp_allocator_handle_t) (omp_high_bw_mem_alloc+1)) align(32) /* OK: omp_low_lat_mem_alloc */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'B' not yet supported" "" { target c } .-1 } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */
+/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'B' not yet supported" "" { target *-*-* } .-1 } */
#pragma omp allocate(C) allocator((omp_allocator_handle_t) 2) /* OK: omp_large_cap_mem_alloc */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */
+/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target *-*-* } .-1 } */
-#pragma omp allocate(A) align(32) allocator(omp_null_allocator) /* { dg-error "'allocator' clause requires a predefined allocator as 'A' is static" "" { xfail c++ } } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
+#pragma omp allocate(A5) align(32) allocator(omp_null_allocator) /* { dg-error "'allocator' clause requires a predefined allocator as 'A5' is static" } */
-#pragma omp allocate(C) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'C' already appeared as list item in an 'allocate' directive" "" { xfail *-*-* } } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-1 } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */
+#pragma omp allocate(C) align(32) allocator(omp_large_cap_mem_alloc)
+/* { dg-error "'C' already appeared as list item in an 'allocate' directive" "" { target c++ } .-1 } */
+/* { dg-message "sorry, unimplemented: '#pragma omp allocate' for static variables like 'C' not yet supported" "" { target c } .-2 } */
// allocate directive in same TU
int f()
{
- #pragma omp allocate(D) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'allocate' directive must be in the same scope as 'D'" "" { xfail c++ } } */
-/* { dg-note "declared here" "" { target c } 21 } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */
- return A[0];
+ #pragma omp allocate(D) align(32) allocator(omp_large_cap_mem_alloc) /* { dg-error "'allocate' directive must be in the same scope as 'D'" } */
+/* { dg-note "declared here" "" { target *-*-* } 25 } */
+ return A1[0];
}
int g()
{
int a2=1, b2=2;
#pragma omp allocate(a2)
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
- #pragma omp allocate(a2) /* { dg-error "'a2' already appeared as list item in an 'allocate' directive" "" { xfail c++ } } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
+ #pragma omp allocate(a2) /* { dg-error "'a2' already appeared as list item in an 'allocate' directive" } */
{
int c2=3;
- #pragma omp allocate(c2, b2) /* { dg-error "'allocate' directive must be in the same scope as 'b2'" "" { xfail c++ } } */
-/* { dg-note "declared here" "" { target c } .-8 } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-2 } */
+ #pragma omp allocate(c2, b2) /* { dg-error "'allocate' directive must be in the same scope as 'b2'" } */
+/* { dg-note "declared here" "" { target *-*-* } .-6 } */
return c2+a2+b2;
}
}
int h(int q)
{
- #pragma omp allocate(q) /* { dg-error "function parameter 'q' may not appear as list item in an 'allocate' directive" "" { xfail c++ } } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
+ #pragma omp allocate(q) /* { dg-error "function parameter 'q' may not appear as list item in an 'allocate' directive" } */
return q;
}
@@ -100,7 +92,6 @@ int
k ()
{
static int var3 = 8;
- #pragma omp allocate(var3) allocator((omp_allocator_handle_t)-1L) /* { dg-error "'allocator' clause requires a predefined allocator as 'var3' is static" "" { target c } } */
-/* { dg-message "sorry, unimplemented: '#pragma omp allocate' not yet supported" "" { target c++ } .-1 } */
+ #pragma omp allocate(var3) allocator((omp_allocator_handle_t)-1L) /* { dg-error "'allocator' clause requires a predefined allocator as 'var3' is static" } */
return var3;
}
diff --git a/gcc/testsuite/g++.dg/gomp/allocate-5.C b/gcc/testsuite/g++.dg/gomp/allocate-5.C
new file mode 100644
index 00000000000..39f423cf794
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gomp/allocate-5.C
@@ -0,0 +1,14 @@
+template<typename t>
+t
+foo()
+{
+ t var = 5;
+ #pragma omp allocate(var) align(sizeof(t) + 1) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */
+ return var;
+}
+
+int
+b()
+{
+ return foo<float>(); /* { dg-message "required from here" } */
+}
diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi
index 02f2d0e5767..315e471e4d4 100644
--- a/libgomp/libgomp.texi
+++ b/libgomp/libgomp.texi
@@ -232,7 +232,7 @@ The OpenMP 4.5 specification is fully supported.
@item Predefined memory spaces, memory allocators, allocator traits
@tab Y @tab See also @ref{Memory allocation}
@item Memory management routines @tab Y @tab
-@item @code{allocate} directive @tab P @tab Only C and Fortran, only stack variables
+@item @code{allocate} directive @tab P @tab Only for stack variables
@item @code{allocate} clause @tab P @tab Initial support
@item @code{use_device_addr} clause on @code{target data} @tab Y @tab
@item @code{ancestor} modifier on @code{device} clause @tab Y @tab
@@ -304,7 +304,7 @@ The OpenMP 4.5 specification is fully supported.
@item @code{strict} modifier in the @code{grainsize} and @code{num_tasks}
clauses of the @code{taskloop} construct @tab Y @tab
@item @code{align} clause in @code{allocate} directive @tab P
- @tab Only C and Fortran (and only stack variables)
+ @tab Only for stack variables
@item @code{align} modifier in @code{allocate} clause @tab Y @tab
@item @code{thread_limit} clause to @code{target} construct @tab Y @tab
@item @code{has_device_addr} clause to @code{target} construct @tab Y @tab
diff --git a/libgomp/testsuite/libgomp.c++/allocate-2.C b/libgomp/testsuite/libgomp.c++/allocate-2.C
new file mode 100644
index 00000000000..f79cada1777
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c++/allocate-2.C
@@ -0,0 +1,329 @@
+/* { dg-do run } */
+/* { dg-additional-options "-fdump-tree-omplower" } */
+
+/* For the 4 vars in omp_parallel, 4 in omp_target and 1 of 2 in each of no_alloc{,2}_func. */
+/* { dg-final { scan-tree-dump-times "__builtin_GOMP_alloc \\(" 10 "omplower" } } */
+/* { dg-final { scan-tree-dump-times "__builtin_GOMP_free \\(" 10 "omplower" } } */
+
+#include <omp.h>
+
+
+void
+check_int (int *x, int y)
+{
+ if (*x != y)
+ __builtin_abort ();
+}
+
+void
+check_ptr (int **x, int *y)
+{
+ if (*x != y)
+ __builtin_abort ();
+}
+
+
+template<typename t>
+t
+no_alloc_func ()
+{
+ /* There is no __builtin_GOMP_alloc / __builtin_GOMP_free as
+ allocator == omp_default_mem_alloc (known at compile time. */
+ t no_alloc, alloc_has_align = 3;
+ #pragma omp allocate(no_alloc) allocator(omp_default_mem_alloc)
+ /* But this one is allocated because of align. */
+ #pragma omp allocate(alloc_has_align) allocator(omp_default_mem_alloc) align(sizeof(t))
+ no_alloc = 7;
+ return no_alloc + alloc_has_align;
+}
+
+template<typename t>
+t
+no_alloc2_func()
+{
+ /* There is no __builtin_GOMP_alloc / __builtin_GOMP_free as
+ no_alloc2 is TREE_UNUSED. But there is for is_alloc2. */
+ t no_alloc2, is_alloc2;
+ #pragma omp allocate(no_alloc2, is_alloc2)
+ is_alloc2 = 7;
+ return is_alloc2;
+}
+
+
+template<typename t>
+void
+omp_parallel ()
+{
+ int n = 6;
+ t iii = 5, jjj[5], kkk[n];
+ t *ptr = (t *) 0x1234;
+ #pragma omp allocate(iii, jjj, kkk, ptr)
+
+ for (int i = 0; i < 5; i++)
+ jjj[i] = 3*i;
+ for (int i = 0; i < 6; i++)
+ kkk[i] = 7*i;
+
+ #pragma omp parallel default(none) firstprivate(iii, jjj, kkk, ptr) if(0)
+ {
+ if (iii != 5)
+ __builtin_abort();
+ iii = 7;
+ check_int (&iii, 7);
+ for (int i = 0; i < 5; i++)
+ if (jjj[i] != 3*i)
+ __builtin_abort ();
+ for (int i = 0; i < 6; i++)
+ if (kkk[i] != 7*i)
+ __builtin_abort ();
+ for (int i = 0; i < 5; i++)
+ jjj[i] = 4*i;
+ for (int i = 0; i < 6; i++)
+ kkk[i] = 8*i;
+ for (int i = 0; i < 5; i++)
+ check_int (&jjj[i], 4*i);
+ for (int i = 0; i < 6; i++)
+ check_int (&kkk[i], 8*i);
+ if (ptr != (int *) 0x1234)
+ __builtin_abort ();
+ ptr = (int *) 0xabcd;
+ if (ptr != (int *) 0xabcd)
+ __builtin_abort ();
+ check_ptr (&ptr, (int *) 0xabcd);
+ }
+ if (iii != 5)
+ __builtin_abort ();
+ check_int (&iii, 5);
+ for (int i = 0; i < 5; i++)
+ {
+ if (jjj[i] != 3*i)
+ __builtin_abort ();
+ check_int (&jjj[i], 3*i);
+ }
+ for (int i = 0; i < 6; i++)
+ {
+ if (kkk[i] != 7*i)
+ __builtin_abort ();
+ check_int (&kkk[i], 7*i);
+ }
+ if (ptr != (int *) 0x1234)
+ __builtin_abort ();
+ check_ptr (&ptr, (int *) 0x1234);
+
+ #pragma omp parallel default(firstprivate) if(0)
+ {
+ if (iii != 5)
+ __builtin_abort();
+ iii = 7;
+ check_int (&iii, 7);
+ for (int i = 0; i < 5; i++)
+ if (jjj[i] != 3*i)
+ __builtin_abort ();
+ for (int i = 0; i < 6; i++)
+ if (kkk[i] != 7*i)
+ __builtin_abort ();
+ for (int i = 0; i < 5; i++)
+ jjj[i] = 4*i;
+ for (int i = 0; i < 6; i++)
+ kkk[i] = 8*i;
+ for (int i = 0; i < 5; i++)
+ check_int (&jjj[i], 4*i);
+ for (int i = 0; i < 6; i++)
+ check_int (&kkk[i], 8*i);
+ if (ptr != (int *) 0x1234)
+ __builtin_abort ();
+ ptr = (int *) 0xabcd;
+ if (ptr != (int *) 0xabcd)
+ __builtin_abort ();
+ check_ptr (&ptr, (int *) 0xabcd);
+ }
+ if (iii != 5)
+ __builtin_abort ();
+ check_int (&iii, 5);
+ for (int i = 0; i < 5; i++)
+ {
+ if (jjj[i] != 3*i)
+ __builtin_abort ();
+ check_int (&jjj[i], 3*i);
+ }
+ for (int i = 0; i < 6; i++)
+ {
+ if (kkk[i] != 7*i)
+ __builtin_abort ();
+ check_int (&kkk[i], 7*i);
+ }
+ if (ptr != (int *) 0x1234)
+ __builtin_abort ();
+ check_ptr (&ptr, (int *) 0x1234);
+}
+
+
+template<typename t>
+void
+omp_target ()
+{
+ int n = 6;
+ t iii = 5, jjj[5], kkk[n];
+ t *ptr = (int *) 0x1234;
+ #pragma omp allocate(iii, jjj, kkk, ptr)
+
+ for (int i = 0; i < 5; i++)
+ jjj[i] = 3*i;
+ for (int i = 0; i < 6; i++)
+ kkk[i] = 7*i;
+
+ #pragma omp target defaultmap(none) firstprivate(iii, jjj, kkk, ptr)
+ {
+ if (iii != 5)
+ __builtin_abort();
+ iii = 7;
+ check_int (&iii, 7);
+ for (int i = 0; i < 5; i++)
+ if (jjj[i] != 3*i)
+ __builtin_abort ();
+ for (int i = 0; i < 6; i++)
+ if (kkk[i] != 7*i)
+ __builtin_abort ();
+ for (int i = 0; i < 5; i++)
+ jjj[i] = 4*i;
+ for (int i = 0; i < 6; i++)
+ kkk[i] = 8*i;
+ for (int i = 0; i < 5; i++)
+ check_int (&jjj[i], 4*i);
+ for (int i = 0; i < 6; i++)
+ check_int (&kkk[i], 8*i);
+ if (ptr != (int *) 0x1234)
+ __builtin_abort ();
+ ptr = (int *) 0xabcd;
+ if (ptr != (int *) 0xabcd)
+ __builtin_abort ();
+ check_ptr (&ptr, (int *) 0xabcd);
+ }
+ if (iii != 5)
+ __builtin_abort ();
+ check_int (&iii, 5);
+ for (int i = 0; i < 5; i++)
+ {
+ if (jjj[i] != 3*i)
+ __builtin_abort ();
+ check_int (&jjj[i], 3*i);
+ }
+ for (int i = 0; i < 6; i++)
+ {
+ if (kkk[i] != 7*i)
+ __builtin_abort ();
+ check_int (&kkk[i], 7*i);
+ }
+ if (ptr != (int *) 0x1234)
+ __builtin_abort ();
+ check_ptr (&ptr, (int *) 0x1234);
+
+ #pragma omp target defaultmap(firstprivate)
+ {
+ if (iii != 5)
+ __builtin_abort();
+ iii = 7;
+ check_int (&iii, 7);
+ for (int i = 0; i < 5; i++)
+ if (jjj[i] != 3*i)
+ __builtin_abort ();
+ for (int i = 0; i < 6; i++)
+ if (kkk[i] != 7*i)
+ __builtin_abort ();
+ for (int i = 0; i < 5; i++)
+ jjj[i] = 4*i;
+ for (int i = 0; i < 6; i++)
+ kkk[i] = 8*i;
+ for (int i = 0; i < 5; i++)
+ check_int (&jjj[i], 4*i);
+ for (int i = 0; i < 6; i++)
+ check_int (&kkk[i], 8*i);
+ if (ptr != (int *) 0x1234)
+ __builtin_abort ();
+ ptr = (int *) 0xabcd;
+ if (ptr != (int *) 0xabcd)
+ __builtin_abort ();
+ check_ptr (&ptr, (int *) 0xabcd);
+ }
+ if (iii != 5)
+ __builtin_abort ();
+ check_int (&iii, 5);
+ for (int i = 0; i < 5; i++)
+ {
+ if (jjj[i] != 3*i)
+ __builtin_abort ();
+ check_int (&jjj[i], 3*i);
+ }
+ for (int i = 0; i < 6; i++)
+ {
+ if (kkk[i] != 7*i)
+ __builtin_abort ();
+ check_int (&kkk[i], 7*i);
+ }
+ if (ptr != (int *) 0x1234)
+ __builtin_abort ();
+ check_ptr (&ptr, (int *) 0x1234);
+
+ #pragma omp target defaultmap(tofrom)
+ {
+ if (iii != 5)
+ __builtin_abort();
+ iii = 7;
+ check_int (&iii, 7);
+ for (int i = 0; i < 5; i++)
+ if (jjj[i] != 3*i)
+ __builtin_abort ();
+ for (int i = 0; i < 6; i++)
+ if (kkk[i] != 7*i)
+ __builtin_abort ();
+ for (int i = 0; i < 5; i++)
+ jjj[i] = 4*i;
+ for (int i = 0; i < 6; i++)
+ kkk[i] = 8*i;
+ for (int i = 0; i < 5; i++)
+ check_int (&jjj[i], 4*i);
+ for (int i = 0; i < 6; i++)
+ check_int (&kkk[i], 8*i);
+ if (ptr != (int *) 0x1234)
+ __builtin_abort ();
+ ptr = (int *) 0xabcd;
+ if (ptr != (int *) 0xabcd)
+ __builtin_abort ();
+ check_ptr (&ptr, (int *) 0xabcd);
+ }
+
+ if (iii != 7)
+ __builtin_abort ();
+ check_int (&iii, 7);
+ for (int i = 0; i < 5; i++)
+ {
+ if (jjj[i] != 4*i)
+ __builtin_abort ();
+ check_int (&jjj[i], 4*i);
+ }
+ for (int i = 0; i < 6; i++)
+ {
+ if (kkk[i] != 8*i)
+ __builtin_abort ();
+ check_int (&kkk[i], 8*i);
+ }
+ if (ptr != (int *) 0xabcd)
+ __builtin_abort ();
+ check_ptr (&ptr, (int *) 0xabcd);
+}
+
+int
+foo()
+{
+ return no_alloc_func<int>() + no_alloc2_func<int>();
+}
+
+int
+main ()
+{
+ omp_parallel<int> ();
+ omp_target<int> ();
+ if (foo() != 10 + 7)
+ __builtin_abort ();
+ return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/allocate-4.c b/libgomp/testsuite/libgomp.c-c++-common/allocate-4.c
similarity index 95%
rename from libgomp/testsuite/libgomp.c/allocate-4.c
rename to libgomp/testsuite/libgomp.c-c++-common/allocate-4.c
index e81cc4093aa..706c8510b84 100644
--- a/libgomp/testsuite/libgomp.c/allocate-4.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/allocate-4.c
@@ -1,6 +1,3 @@
-/* TODO: move to ../libgomp.c-c++-common once C++ is implemented. */
-/* NOTE: { target c } is unsupported with with the C compiler. */
-
/* { dg-do run } */
/* { dg-additional-options "-fdump-tree-gimple" } */
diff --git a/libgomp/testsuite/libgomp.c/allocate-5.c b/libgomp/testsuite/libgomp.c-c++-common/allocate-5.c
similarity index 96%
rename from libgomp/testsuite/libgomp.c/allocate-5.c
rename to libgomp/testsuite/libgomp.c-c++-common/allocate-5.c
index beaf16440e1..3bbe78db840 100644
--- a/libgomp/testsuite/libgomp.c/allocate-5.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/allocate-5.c
@@ -1,6 +1,3 @@
-/* TODO: move to ../libgomp.c-c++-common once C++ is implemented. */
-/* NOTE: { target c } is unsupported with with the C compiler. */
-
/* { dg-do run } */
/* { dg-additional-options "-fdump-tree-gimple" } */
diff --git a/libgomp/testsuite/libgomp.c/allocate-6.c b/libgomp/testsuite/libgomp.c-c++-common/allocate-6.c
similarity index 98%
rename from libgomp/testsuite/libgomp.c/allocate-6.c
rename to libgomp/testsuite/libgomp.c-c++-common/allocate-6.c
index 6d7278ce571..669581b327d 100644
--- a/libgomp/testsuite/libgomp.c/allocate-6.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/allocate-6.c
@@ -1,6 +1,3 @@
-/* TODO: move to ../libgomp.c-c++-common once C++ is implemented. */
-/* NOTE: { target c } is unsupported with with the C compiler. */
-
/* { dg-do run } */
/* { dg-additional-options "-fdump-tree-omplower" } */
^ permalink raw reply [flat|nested] 3+ messages in thread
* *ping* / [Patch] OpenMP: Add C++ support for 'omp allocate' with stack variables
2023-10-20 16:49 [Patch] OpenMP: Add C++ support for 'omp allocate' with stack variables Tobias Burnus
@ 2023-11-07 8:14 ` Tobias Burnus
2023-12-08 11:28 ` Jakub Jelinek
1 sibling, 0 replies; 3+ messages in thread
From: Tobias Burnus @ 2023-11-07 8:14 UTC (permalink / raw)
To: gcc-patches, Jakub Jelinek
As I know my lack of C++ FE knowledge, I would appreciate very much if
someone could have a look to reduce the chance that I did something
stupid or missed something obvious...
https://gcc.gnu.org/pipermail/gcc-patches/2023-October/633782.html
Tobias
On 20.10.23 18:49, Tobias Burnus wrote:
> This patch adds C++ support for OpenMP's 'omp allocate' for
> stack/automatic arrays.
>
> Comments and suggestions? — I bet there are given my little knowledge
> about the C++ FE...
>
> Tobias
>
> PS: I think I should write some additional C++-specific code, I bet some
> corner cases are missed. The question is only which ones.
> -----------------
> 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
-----------------
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch] OpenMP: Add C++ support for 'omp allocate' with stack variables
2023-10-20 16:49 [Patch] OpenMP: Add C++ support for 'omp allocate' with stack variables Tobias Burnus
2023-11-07 8:14 ` *ping* / " Tobias Burnus
@ 2023-12-08 11:28 ` Jakub Jelinek
1 sibling, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2023-12-08 11:28 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gcc-patches
On Fri, Oct 20, 2023 at 06:49:58PM +0200, Tobias Burnus wrote:
> + if (!processing_template_decl)
> + finish_omp_allocate (true, OMP_CLAUSE_LOCATION (nl), var);
The above should be called even if processing_template_decl, see below.
And pass DECL_ATTRIBUTES (var) to it (also see below).
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7801,6 +7801,10 @@ extern tree finish_omp_for (location_t, enum tree_code,
> tree, tree, tree, tree, tree,
> tree, tree, vec<tree> *, tree);
> extern tree finish_omp_for_block (tree, tree);
> +extern void finish_omp_allocate (bool, location_t, tree,
> + tree = NULL_TREE,
> + tsubst_flags_t = tf_warning_or_error,
> + tree = NULL_TREE);
For a function called in 2 spots adding default arguments is an overkill,
but see below.
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -41864,6 +41864,67 @@ cp_parser_omp_structured_block (cp_parser *parser, bool *if_p)
> return finish_omp_structured_block (stmt);
> }
>
Better also add a comment about this structure.
> +struct cp_omp_loc_tree
> +{
> + location_t loc;
> + tree var;
> +};
> +
> +/* Check whether the expression used in the allocator clause is declared or
> + modified between the variable declaration and its allocate directive. */
> +static tree
> +cp_check_omp_allocate_allocator_r (tree *tp, int *, void *data)
> +{
> + tree var = ((struct cp_omp_loc_tree *) data)->var;
> + location_t loc = ((struct cp_omp_loc_tree *) data)->loc;
> + tree v = NULL_TREE;
> + if (TREE_CODE (*tp) == VAR_DECL)
VAR_P ?
> + for (v = current_binding_level->names; v; v = TREE_CHAIN (v))
> + if (v == var)
> + break;
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 210c6cb9e4d..10603f4c39f 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -18379,6 +18379,10 @@ tsubst_stmt (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>
> cp_finish_decl (decl, init, const_init, asmspec_tree, 0,
> decomp);
> +
> + if (flag_openmp && VAR_P (decl))
> + finish_omp_allocate (false, DECL_SOURCE_LOCATION (decl),
> + decl, args, complain, in_decl);
The normal C++ FE way of doing stuff is perform all the substitutions
here (in tsubst_stmt, tsubst_expr and functions it calls) and then call
the various semantics.cc etc. finalizers, with already tsubsted arguments.
So, this would be the first time to do it differently.
IMHO better lookup_attribute here, if found tsubst what is needed and
only then call finish_omp_allocate. Ideally when you've done the work
already pass the attr to the function as well.
> if (ndecl != error_mark_node)
> cp_finish_decomp (ndecl, decomp);
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index dc3c11461fb..30c70c0b13e 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -10987,6 +10987,86 @@ finish_omp_for_block (tree bind, tree omp_for)
> return bind;
> }
>
Please add a function comment.
> +void
> +finish_omp_allocate (bool in_parsing, location_t loc, tree decl, tree args,
> + tsubst_flags_t complain, tree in_decl)
As mentioned above, please drop the in_parsing, args, complain and in_decl
arguments and add attr.
> +{
> + location_t loc2;
> + tree attr = lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl));
> + if (attr == NULL_TREE)
> + return;
> +
> + tree allocator = TREE_PURPOSE (TREE_VALUE (attr));
> + tree alignment = TREE_VALUE (TREE_VALUE (attr));
> +
> + if (alignment == error_mark_node)
> + TREE_VALUE (TREE_VALUE (attr)) = NULL_TREE;
> + else if (alignment)
> + {
> + location_t loc2 = EXPR_LOCATION (alignment);
> + if (!in_parsing)
> + alignment = tsubst_expr (alignment, args, complain, in_decl);
> + alignment = fold_non_dependent_expr (alignment);
> +
> + if (TREE_CODE (alignment) != INTEGER_CST
> + || !INTEGRAL_TYPE_P (TREE_TYPE (alignment))
Please see e.g. the r13-8124 and r14-6193 changes. Unless we have (possibly
violating standard?) hard restriction like we have on the collapse and
ordered clauses where we want the argument to be INTEGER_CST with the right
value already during parsing because parsing depends on it, we want to
handle 3 different cases, the directive appearing in non-template code,
in that case the diagnostics should be done right away, or in template code
where the arguments of the clauses etc. are not dependent (type nor value),
in that case we want the diagnostics to be also done at parsing time; not
strictly required by the standard, but QoI, get diagnostics of something
clearly incorrect in a template even when the template will never be
instantiated. And finally if it is dependent (value dependent like say in
template <int N, typename T, T X>
N, in that case we can diagnose at parsing time if the type is not integer
type, but shouldn't diagnose anything about the value, or say for X
above which is type dependent, then we shouldn't diagnose anything for it).
Now, typically the finish_* and similar routines in semantics.cc etc.
for processing_template_decl construct some tree that is later tsubsted,
and often if it is just value dependent and not type dependent ensure
it has the right finalized type; that part isn't needed in this case,
when it is already the parser caller (and should be the pt.cc caller) which
arranges stuff to be in the attribute.
> + || tree_int_cst_sgn (alignment) != 1
> + || !integer_pow2p (alignment))
So, you want to just store the alignment into TREE_VALUE (TREE_VALUE (attr))
if it is type_dependent_expression_p so that later instantiation handles
that, or error if it is !INTEGRAL_TYPE_P otherwise. Otherwise
if value_dependent_expression_p again just store it, or otherwise
do the INTEGER_CST and exact value checks and error otherwise.
> + {
> + error_at (loc2, "%<align%> clause argument needs to be positive "
> + "constant power of two integer expression");
> + TREE_VALUE (TREE_VALUE (attr)) = NULL_TREE;
I'd argue that you want at least when processing_template_decl
store there error_mark_node, to differentiate the cases between
it has been erroneous and already diagnosed vs. it wasn't specified
at all. For the former, you don't want to error on it in any way
further. Of course, perhaps for middle-end you want to ensure the
value is INTEGER_CST and valid only, in such a case you'd then
change the error_mark_nodes to e.g. NULL_TREEs or drop the attribute
altogether at !processing_template_decl time only.
> + allocator = fold_non_dependent_expr (allocator);
I don't understand this, you correctly attempt to fold it into
a constant first.
> + if (allocator)
> + {
> + tree orig_type = TYPE_MAIN_VARIANT (TREE_TYPE (allocator));
> + if (!INTEGRAL_TYPE_P (TREE_TYPE (allocator))
> + || TREE_CODE (orig_type) != ENUMERAL_TYPE
> + || TYPE_NAME (orig_type) == NULL_TREE
Here see above comments again.
> + || (DECL_NAME (TYPE_NAME (orig_type))
> + != get_identifier ("omp_allocator_handle_t")))
> + {
> + error_at (loc2, "%<allocator%> clause expression has type "
> + "%qT rather than %<omp_allocator_handle_t%>",
> + TREE_TYPE (allocator));
> + allocator = TREE_PURPOSE (TREE_VALUE (attr)) = NULL_TREE;
> + }
> + else
> + TREE_PURPOSE (TREE_VALUE (attr)) = fold_non_dependent_expr (allocator);
But then fold it again? This is useless.
> + }
> + if (TREE_STATIC (decl))
> + {
> + if (allocator == NULL_TREE)
> + error_at (loc, "%<allocator%> clause required for "
> + "static variable %qD", decl);
See above comments about differentiating something we've diagnosed already
from what you want to diagnose here, missing clause.
> + else if (allocator
> + && (wi::to_widest (allocator) < 1
> + || wi::to_widest (allocator) > 8))
And this again should be done only if allocator is not
value_dependent_expression_p, otherwise neither the error nor sorry.
> + /* 8 = largest predefined memory allocator. */
> + error_at (loc2, "%<allocator%> clause requires a predefined allocator "
> + "as %qD is static", decl);
> + else
> + sorry_at (loc, "%<#pragma omp allocate%> for static variables like %qD "
> + "not yet supported", decl);
> + }
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/gomp/allocate-5.C
> @@ -0,0 +1,14 @@
> +template<typename t>
> +t
> +foo()
> +{
> + t var = 5;
> + #pragma omp allocate(var) align(sizeof(t) + 1) /* { dg-error "'align' clause argument needs to be positive constant power of two integer expression" } */
You want to cover in the testsuite all the cases I've described
above, non-template valid and invalid values, template where
the values are not dependent and do fold there to INTEGER_CSTs,
again valid and invalid values, then when the values are value
dependent but have valid/invalid types and finally when they
are type dependent. And have cases where such templates are
never instantiated (then check for diagnostics that can be done
on such templates or has to be deferred), and have different
templates instantiated with something that is valid and then
others with something that makes it invalid.
Hope this helps.
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-12-08 11:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-20 16:49 [Patch] OpenMP: Add C++ support for 'omp allocate' with stack variables Tobias Burnus
2023-11-07 8:14 ` *ping* / " Tobias Burnus
2023-12-08 11:28 ` 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).