Due to the nature of transformations made on vars in an 'omp allocate'
directive, there are additional requirements on an allocator clause
attached to the directive. At minimum, all vars used in its expr must be
declared before all the vars specified in the allocate directive, this
also includes vars used in the directive themselves. This is of course
unless the vars in the expr are unevaluated or static constexpr. We could
potentially allow static but it doesn't seem practical. Allowing constexpr
vars without static could also be fine if the clause is constant evaluated
but that increases complexity. One can still declare such vars before the
first var in the allocate directive.
This patch diagnoses such cases, and takes into account those exceptions.
In the future we could provide better diagnostics for the edge cases, such
as suggesting adding 'static' to a constexpr variable declaration.
The C front end also tries to diagnose mutations of such vars that occur
between the first declared variable in the allocate directive and the
allocate directive itself, but it misses non-trivial cases. This patch
does not attempt to diagnose these. It would be better to move such
diagnostics and warnings into finish_omp_allocate as there are a lot of
cases that are dependent. The diagnostics of declaration order and bad uses
of vars can stay in cp_parser_omp_allocate.
gcc/cp/ChangeLog:
* parser.cc (cp_parser_omp_allocate): Add allocator clause expr
diagnostics.
gcc/testsuite/ChangeLog:
* c-c++-common/gomp/allocate-5.c: Adjust test.
* c-c++-common/gomp/allocate-12.c: Remove xfail.
* c-c++-common/gomp/allocate-16.c: Likewise.
* c-c++-common/gomp/allocate-20.c: Likewise.
* g++.dg/gomp/allocate-23.C: New test.
Signed-off-by: Waffl3x <[email protected]>
---
gcc/cp/parser.cc | 129 ++++++++++++++++++
gcc/testsuite/c-c++-common/gomp/allocate-12.c | 6 +-
gcc/testsuite/c-c++-common/gomp/allocate-16.c | 9 +-
gcc/testsuite/c-c++-common/gomp/allocate-20.c | 12 +-
gcc/testsuite/c-c++-common/gomp/allocate-5.c | 5 +-
gcc/testsuite/g++.dg/gomp/allocate-23.C | 24 ++++
6 files changed, 170 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/gomp/allocate-23.C
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 412a71c439c..aa6777e5ed4 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -47190,6 +47190,10 @@ cp_parser_omp_allocate (cp_parser *parser, cp_token
*pragma_tok)
nl = nreverse (nl);
const tree directive_ctx = current_scope ();
+ /* Used for diagnostics of the allocator clause's expr in
+ check_omp_allocate_allocator_r. We only keep this locally, nl is still
+ what ultimately gets passed along to finish_omp_allocate. */
+ hash_map<tree, location_t> arg_map;
{
auto var_is_in_scope = [&] (tree var_decl)
{
@@ -47325,6 +47329,7 @@ cp_parser_omp_allocate (cp_parser *parser, cp_token
*pragma_tok)
arg_loc_wrapper,
*attr_chain);
}
+ arg_map.put (var, arg_loc);
/* Keep the node. */
chain = &TREE_CHAIN (node);
}
@@ -47373,6 +47378,130 @@ cp_parser_omp_allocate (cp_parser *parser, cp_token
*pragma_tok)
} while (true);
cp_parser_require_pragma_eol (parser, pragma_tok);
+ /* Used in check_omp_allocate_allocator_r callback. */
+ struct omp_alloc_expr_ctx
+ {
+ location_t alloc_expr_loc;
+ tree first_arg;
+ hash_map<tree, location_t>& arg_map;
+ hash_set<tree>& declared_after_first_arg;
+ /* Vars used erroneously in the alloc expr. */
+ hash_set<tree> diagnosed;
+ };
+ /* Callback for cp_walk_tree. Always returns NULL_TREE so we diagnose as
+ much as possible. */
+ auto check_omp_allocate_allocator_r = [] (tree *tp, int *ws, void *data)
+ {
+ gcc_assert (tp != nullptr && ws != nullptr && data != nullptr);
+ /* Uses of variables in unevaluated contexts are permitted. */
+ if (TREE_CODE (*tp) == DECLTYPE_TYPE)
+ {
+ *ws = 0;
+ return NULL_TREE;
+ }
+ /* Unwrap manually so we have a location for diagnostics. */
+ tree var_in_expr = tree_strip_any_location_wrapper (*tp);
+ if (!VAR_P (var_in_expr))
+ return NULL_TREE;
+ /* We can technically allow more than this, but it gets a bit more hazy.
+ In particular just 'static' should be okay, but it would need to be
+ constant initialized. It just doesn't make sense to allow it.
+ (It can still be used as long as it is declared before.) */
+ if (DECL_DECLARED_CONSTEXPR_P (var_in_expr)
+ && TREE_STATIC (var_in_expr))
+ return NULL_TREE;
+ /* Don't walk wrappers of vars, unwrap them manually (see above). */
+ if (location_wrapper_p (*tp))
+ *ws = 0;
+
+ omp_alloc_expr_ctx& ctx = *static_cast<omp_alloc_expr_ctx *>(data);
+ /* Don't repeat diagnostics if we find the same var used twice. */
+ if (ctx.diagnosed.contains (var_in_expr))
+ return NULL_TREE;
+
+ /* Don't evaluate the location until we need it. */
+ auto make_alloc_expr_loc = [&] ()
+ {
+ const location_t alloc_expr_loc = ctx.alloc_expr_loc;
+ const location_t loc = EXPR_LOCATION (*tp);
+ /* If *tp has a meaningful location, use it for the caret. */
+ return loc == UNKNOWN_LOCATION
+ ? alloc_expr_loc
+ : make_location (loc, alloc_expr_loc, alloc_expr_loc);
+ };
+
+ if (const location_t *const arg_loc = ctx.arg_map.get (var_in_expr))
+ {
+ auto_diagnostic_group d;
+ error_at (make_alloc_expr_loc (),
+ "variable %qD used in %<allocate%> directive must "
+ "not be used in its %<allocator%> clause", var_in_expr);
+ inform (DECL_SOURCE_LOCATION (var_in_expr), "declared here");
+ inform (*arg_loc,
+ "used in allocate directive here");
+ ctx.diagnosed.add (var_in_expr);
+ return NULL_TREE;
+ }
+ if (ctx.declared_after_first_arg.contains (var_in_expr))
+ {
+ auto_diagnostic_group d;
+ error_at (make_alloc_expr_loc (),
+ "variable %qD used in the %<allocator%> clause "
+ "must be declared before %qD",
+ var_in_expr, ctx.first_arg);
+ inform (DECL_SOURCE_LOCATION (var_in_expr), "declared here");
+ inform (DECL_SOURCE_LOCATION (ctx.first_arg),
+ "first to be allocated variable declared here");
+ ctx.diagnosed.add (var_in_expr);
+ return NULL_TREE;
+ }
+ return NULL_TREE;
+ };
+
+ /* Diagnose invalid uses of variables in the allocator clause's expr.
+ If we want to warn about variables that are potentially modified, either
+ by their address being taken or a reference being bound, it must be done
+ in cp/semantics.cc:finish_omp_allocate instead as such cases can
+ potentially be dependent. */
+ if (allocator != NULL_TREE && !arg_map.is_empty ())
+ {
+ hash_set<tree> declared_after_first_arg;
+ const tree first_arg = [&] ()
+ {
+ tree var = current_binding_level->names;
+ size_t remaining_args = arg_map.elements ();
+ for (; var != NULL_TREE; var = DECL_CHAIN (var))
+ {
+ gcc_assert (remaining_args != 0);
+ if (!VAR_P (var))
+ continue;
+ if (!arg_map.get (var))
+ {
+ /* We haven't seen every arg in arg_map, this var is declared
+ after at least one of the args. */
+ declared_after_first_arg.add (var);
+ continue;
+ }
+ --remaining_args;
+ if (remaining_args == 0)
+ break;
+ }
+ /* We stop the above loop before advancing the chain so VAR is the
+ first variable declared (not to be confused with the first arg)
+ that was passed into the allocate directive. */
+ return var;
+ } (); /* IILE. */
+
+ gcc_assert (first_arg != NULL_TREE && arg_map.get (first_arg));
+ omp_alloc_expr_ctx ctx = {allocator.get_location (), first_arg, arg_map,
+ declared_after_first_arg, hash_set<tree>()};
+ tree a = allocator.get_value ();
+ cp_walk_tree (&a, check_omp_allocate_allocator_r, &ctx, nullptr);
+ /* Don't try to do anything else with an invalid allocator expr. */
+ if (!ctx.diagnosed.is_empty ())
+ allocator = cp_expr (error_mark_node, UNKNOWN_LOCATION);
+ }
+
/* Some codes, such as template parameters, don't get wrapped by
maybe_wrap_with_location despite not being able to carry a location.
We need a location to issue good diagnostics in finish_omp_allocate. */
diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-12.c
b/gcc/testsuite/c-c++-common/gomp/allocate-12.c
index 6a53770697c..6f3ddd361f2 100644
--- a/gcc/testsuite/c-c++-common/gomp/allocate-12.c
+++ b/gcc/testsuite/c-c++-common/gomp/allocate-12.c
@@ -47,9 +47,9 @@ f2 ()
int
g ()
{
- int n = 5; /* { dg-note "to be allocated variable declared here" "" { xfail
c++ } } */
- omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; /* { dg-note
"declared here" "" { xfail c++ } } */
- #pragma omp allocate(n) allocator(my_allocator) /* { dg-error "variable
'my_allocator' used in the 'allocator' clause must be declared before 'n'" "" {
xfail c++ } } */
+ int n = 5; /* { dg-note "to be allocated variable declared here" } */
+ omp_allocator_handle_t my_allocator = omp_low_lat_mem_alloc; /* { dg-note
"declared here" } */
+ #pragma omp allocate(n) allocator(my_allocator) /* { dg-error "variable
'my_allocator' used in the 'allocator' clause must be declared before 'n'" } */
n = 7;
return n;
}
diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-16.c
b/gcc/testsuite/c-c++-common/gomp/allocate-16.c
index 7df9a92fd9f..b5317b575e5 100644
--- a/gcc/testsuite/c-c++-common/gomp/allocate-16.c
+++ b/gcc/testsuite/c-c++-common/gomp/allocate-16.c
@@ -14,15 +14,16 @@ omp_allocator_handle_t foo(int, int *);
void
f ()
{
- int v; /* { dg-note "to be allocated variable declared here" "" { xfail c++
} } */
+ int v; /* { dg-note "to be allocated variable declared here" } */
static const int n = 5;
int a = 1;
- /* { dg-note "declared here" "" { xfail c++ } .-1 } */
+ /* { dg-note "declared here" "" { target *-*-* } .-1 } */
int b[n];
- /* { dg-note "declared here" "" { target c++ xfail c++ } .-1 } */
+ /* { dg-note "declared here" "" { target c++ } .-1 } */
b[a] = 5;
#pragma omp allocate (v) allocator (foo (a, &b[a]))
- /* { dg-error "variable 'a' used in the 'allocator' clause must be declared
before 'v'" "" { xfail c++ } .-1 } */
+ /* { dg-error "variable 'a' used in the 'allocator' clause must be declared
before 'v'" "" { target *-*-* } .-1 } */
+ /* { dg-error "variable 'b' used in the 'allocator' clause must be declared
before 'v'" "" { target c++ } .-2 } */
}
void
diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-20.c
b/gcc/testsuite/c-c++-common/gomp/allocate-20.c
index 604e4847a92..36f3cb3627c 100644
--- a/gcc/testsuite/c-c++-common/gomp/allocate-20.c
+++ b/gcc/testsuite/c-c++-common/gomp/allocate-20.c
@@ -262,8 +262,8 @@ void f_with_parm_and_allocator0(int p) /* { dg-note
"parameter 'p' declared here
void f_with_parm_and_allocator1(int p) /* { dg-note "parameter 'p' declared
here" "" { target c++ } } */
{
- int v0; /* { dg-note "to be allocated variable declared here" "" { xfail c++
} } */
- omp_allocator_handle_t alloc0 = omp_default_mem_alloc; /* { dg-note
"declared here" "" { xfail c++ } } */
+ int v0; /* { dg-note "to be allocated variable declared here" } */
+ omp_allocator_handle_t alloc0 = omp_default_mem_alloc; /* { dg-note
"declared here" } */
/* { dg-error "function parameter 'p' may not appear as list item in an
'allocate' directive" "" { target c } .+2 } */
/* { dg-error "variable 'alloc0' used in the 'allocator' clause must be
declared before 'v0'" "" { target c } .+1 } */
#pragma omp allocate(\
@@ -271,7 +271,7 @@ void f_with_parm_and_allocator1(int p) /* { dg-note
"parameter 'p' declared here
v0)\
allocator(alloc0)
/* { dg-error "function parameter 'p' may not appear as list item in an
'allocate' directive" "" { target c++ } .-3 } */
- /* { dg-error "variable 'alloc0' used in the 'allocator' clause must be
declared before 'v0'" "" { target c++ xfail c++ } .-2 } */
+ /* { dg-error "variable 'alloc0' used in the 'allocator' clause must be
declared before 'v0'" "" { target c++ } .-2 } */
int v1; /* { dg-note "declared here" } */
{
@@ -285,8 +285,8 @@ void f_with_parm_and_allocator1(int p) /* { dg-note
"parameter 'p' declared here
/* { dg-error "'allocate' directive must be in the same scope as 'v1'" ""
{ target c++ } .-3 } */
}
{
- int v3; /* { dg-note "to be allocated variable declared here" "" { xfail
c++ } } */
- omp_allocator_handle_t alloc2 = omp_default_mem_alloc; /* { dg-note
"declared here" "" { xfail c++ } } */
+ int v3; /* { dg-note "to be allocated variable declared here" } */
+ omp_allocator_handle_t alloc2 = omp_default_mem_alloc; /* { dg-note
"declared here" } */
/* { dg-error "variable 'alloc2' used in the 'allocator' clause must be
declared before 'v3'" "" { target c } .+2 } */
/* { dg-error "'allocate' directive must be in the same scope as 'v1'" ""
{ target c } .+1 } */
#pragma omp allocate(\
@@ -294,7 +294,7 @@ void f_with_parm_and_allocator1(int p) /* { dg-note
"parameter 'p' declared here
v3\
) allocator(alloc2)
/* { dg-error "'allocate' directive must be in the same scope as 'v1'" ""
{ target c++ } .-3 } */
- /* { dg-error "variable 'alloc2' used in the 'allocator' clause must be
declared before 'v3'" "" { target c++ xfail c++ } .-2 } */
+ /* { dg-error "variable 'alloc2' used in the 'allocator' clause must be
declared before 'v3'" "" { target c++ } .-2 } */
}
}
diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-5.c
b/gcc/testsuite/c-c++-common/gomp/allocate-5.c
index b34f9ea9227..e47bfb217bf 100644
--- a/gcc/testsuite/c-c++-common/gomp/allocate-5.c
+++ b/gcc/testsuite/c-c++-common/gomp/allocate-5.c
@@ -39,9 +39,10 @@ bar ()
/* { dg-error "expected end of line before '\\(' token" "" { target *-*-* }
.-1 } */
#pragma omp allocate(a2) allocator(b)
/* { dg-error "'allocator' clause expression has type 'int' rather than
'omp_allocator_handle_t'" "" { target c } .-1 } */
- /* { dg-error "invalid conversion from 'int' to 'omp_allocator_handle_t'" ""
{ target c++ } .-2 } */
+ /* { dg-error "variable 'b' used in the 'allocator' clause must be declared
before 'a2'" "" { target c++ } .-2 } */
/* We have diverging behavior here between c and c++ due to a difference in
- order of diagnostics, as well as diverging semantics, this should
probably be unified. */
+ order of diagnostics, as well as diverging semantics, this should
probably be unified.
+ Really this is probably very bugged in C. */
}
diff --git a/gcc/testsuite/g++.dg/gomp/allocate-23.C
b/gcc/testsuite/g++.dg/gomp/allocate-23.C
new file mode 100644
index 00000000000..8593dfcc915
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gomp/allocate-23.C
@@ -0,0 +1,24 @@
+/* { dg-do compile { target c++14 } } */
+#include "allocate-allocator-handle.h"
+
+/* Valid uses of vars in an allocator clause. */
+
+void constexpr_var_declared_after()
+{
+ int a = 42;
+ static constexpr omp_allocator_handle_t my_handle = omp_default_mem_alloc;
+ #pragma omp allocate(a) allocator(my_handle)
+}
+
+template<typename...>
+constexpr omp_allocator_handle_t get_alloc()
+{
+ return omp_default_mem_alloc;
+}
+
+void unevaluated_use_of_var()
+{
+ int a = 42;
+ int b = 42;
+ #pragma omp allocate(a, b) allocator(get_alloc<decltype(a), decltype(b)>())
+}
--
2.54.0
