Sandra Loosemore wrote:
This patch implements C++ support for the "begin declare variant"
construct. The OpenMP specification is hazy on interaction of this
feature with C++ language features. Variant functions in classes are
supported but must be defined as members in the class definition,
using an unqualified name for the base function which also must be
present in that class. Similarly variant functions in a namespace can
only be defined in that namespace using an unqualified name for a base
function already declared in that namespace. Variants for template
functions or inside template classes seem to (mostly) work.
* * *
For completeness, the PR mentioned in delim-declare-variant-{40,81}.C
is the still to be fixed
PR118530 - [OpenMP] declare_variant - non-arg variant with
non-template return value type not selected
(The tests use xfail, i.e. will show up as xpass once fixed.)
* * *
+omp_finish_variant_function (cp_parser *parser, tree decl, tree base_name,
+ tree ctx, bool in_class_p, bool diagnose_error_p)
+{
...
+ tree attrs = DECL_ATTRIBUTES (match);
+ tree match_loc_node
+ = maybe_wrap_with_location (integer_zero_node,
+ DECL_SOURCE_LOCATION (match));
Can this be '= build_empty_stmt (DECL_SOURCE_LOCATION (match));'
instead?
* * *
+ /* Except for variants defined in a module interface, variant functions
+ are essentially anonymous and cannot be referenced by name, so make
+ them have internal linkage. Note that class methods in C++ normally
+ have external linkage with weak/comdat semantics; this prevents that. */
+ if (!module_interface_p ())
+ {
+ TREE_PUBLIC (decl) = 0;
+ DECL_COMDAT (decl) = 0;
+ DECL_INTERFACE_KNOWN (decl) = 1;
+ DECL_NOT_REALLY_EXTERN (decl) = 1;
+ }
I guess it is okay to export the variant even if the base isn't:
static void foo();
#pragma omp begin declare variant match(…)
void foo(); // no static
#pragma end declare variant
Because that happens here. I think that's somewhat consistent,
but maybe, e.g., if (... || !TREE_PUBLIC (base_decl)) makes sense here?
* * *
BTW: I think the following is invalid - but it is not diagnosed.
constexpr int get_number () { return 1; }
#pragma omp begin declare variant match(implementation={vendor("gnu")})
constexpr int get_number () { return 42; }
#pragma omp end declare variant
static int v[get_number ()];
static_assert (sizeof(v) == sizeof (int) * 42);
// Assert fails as no variant substitution happens
I am not sure whether that counts as immediate function
– but the problem is the same as for immediate functions.
I guess, we want to open a PR about this - and probably discuss
this at OpenMP spec level.
* * *
+++ b/gcc/testsuite/g++.dg/gomp/delim-declare-variant-4.C
@@ -0,0 +1,57 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fdump-tree-gimple" } */
...
+int main (void)
+{
+ if (n1::foo (42) != 42) __builtin_abort ();
+ if (n1::bar (3) != 12) __builtin_abort ();
+#pragma omp target
+ {
+ if (n1::foo (42) != 43) __builtin_abort ();
+ if (n1::bar (3) != 12) __builtin_abort ();
+ }
+}
...
+/* { dg-final { scan-tree-dump-times "bar\\.ompvariant. \\(3\\)" 2 "gimple" }
} */
I wonder whether we should also check that the right 'bar' is used,
e.g. -fdump-tree-optimized (with -O > 0) does this. (Applies e.g. also
to the next one.)
On the other hand, I noticed that the runtime test happens in
testsuite/libgomp.c++/delim-declare-variant-1.C (etc.).
* * *
+++ b/gcc/testsuite/g++.dg/gomp/delim-declare-variant-6.C
+/* Make sure all the template functions are instantiated. */
+/* { dg-final { scan-tree-dump "int foo.ompvariant.<int> \\(.*\\)" "gimple" } }
+/* { dg-final { scan-tree-dump "int foo<int> \\(.*\\)" "gimple" } }
+/* { dg-final { scan-tree-dump "int bar.ompvariant.<int> \\(.*\\)" "gimple" } }
Can you either use // or add a couple of */ ?
* * *
All in all: LGTM.
Thanks,
Tobias
