Hi Sandra, hello world,
see other email from today in this thread regarding a C++
module test case + C++ module issues and a parser issues.
Regarding the patch itself:
On February 10, 2025, Sandra Loosemore wrote:
This patch adds functions for variant name mangling and context selector
merging that are shared by the C and C++ front ends.
The OpenMP specification says that name mangling is supposed to encode
the context selector for the variant, but also provides for no way to
reference these functions directly by name or from a different
compilation unit. It also gives no guidance on how dynamic selectors
might be encoded across compilation units.
The GCC implementation of this feature instead treats variant
functions as if they have no linkage and uses a simple counter to
generate names.
The wording needs to be updated for C++ modules. The counting can be
done identically - as the name gets the module name appended,
however, it can be externally accessed (if and only if exported).
gcc/ChangeLog
* omp-general.cc (omp_mangle_variant_name): New.
(omp_check_for_duplicate_variant): New.
(omp_copy_trait_set): New.
(omp_trait_selectors_equivalent): New.
(omp_combine_trait_sets): New.
(omp_merge_context_selectors): New.
* omp-general.h (omp_mangle_variant_name): Declare.
(omp_check_for_duplicate_variant): Declare.
(omp_merge_context_selectors): Declare.
---
gcc/omp-general.cc | 194 +++++++++++++++++++++++++++++++++++++++++++++
gcc/omp-general.h | 5 ++
2 files changed, 199 insertions(+)
diff --git a/gcc/omp-general.cc b/gcc/omp-general.cc
index 0a2dd6b5be7..c7b8e17d1f7 100644
--- a/gcc/omp-general.cc
+++ b/gcc/omp-general.cc
@@ -1501,6 +1501,66 @@ omp_check_context_selector (location_t loc, tree ctx,
return ctx;
}
+/* Produce a mangled version of BASE_ID for the name of the variant
+ function with context selector CTX. SEP is a separator string.
+ The return value is an IDENTIFIER_NODE.
+
+ Per the OpenMP spec, "the symbol names of two definitions of a function are
+ considered to be equal if and only if their effective context selectors are
+ equivalent". However, if we did have two such definitions, we'd get an ODR
+ violation. We already take steps in the front ends to make variant
+ functions internal to the compilation unit, since there is no (portable) way
+ to reference them directly by name or declare them as extern in another
+ compilation unit. So, we can diagnose the would-be ODR violations by
+ checking that there is not already a variant for the same function with an
+ equivalent context selector, and otherwise just use a simple counter to name
+ the variant functions instead of any complicated scheme to encode the
+ context selector in the name. */
+
Likewise.
(That 'begin/end declare variant' mangling only applies to function
definitions and not function declarations, which also prevents using
them outside either a local variant or a C++ module.)
+tree
+omp_mangle_variant_name (tree base_id, tree ctx ATTRIBUTE_UNUSED,
+ const char *sep)
+{
+ const char *base_name = IDENTIFIER_POINTER (base_id);
+
+ /* Now do the actual mangling. */
+ static int variant_counter;
+ /* The numeric suffix and terminating byte ought to need way less than
+ 32 bytes extra, that's just a magic number. */
+ size_t buflen = (strlen (base_name) + strlen (sep) + strlen ("ompvariant")
+ + 32);
+ char *buffer = (char *) alloca (buflen);
+ snprintf (buffer, buflen, "%s%sompvariant%d", base_name, sep,
+ ++variant_counter);
+ return get_identifier (buffer);
How about:
buflen = snprintf (...)
returnget_identifier_with_length (buffer, buflen); as tiny optimization?
+/* Diagnose an error if there is already a variant with CTX registered
+ for BASE_DECL. Returns true if OK, false otherwise. */
+bool
+omp_check_for_duplicate_variant (location_t loc, tree base_decl, tree ctx)
+{
+ for (tree attr = DECL_ATTRIBUTES (base_decl); attr; attr = TREE_CHAIN (attr))
+ {
+ attr = lookup_attribute ("omp declare variant base", attr);
+ if (attr == NULL_TREE)
+ break;
Wouldn't it be simpler and cleaner to just write:
for (tree attr = lookup_attribute ("...", DECL_ATTRIBUTES (base_decl);
attr; attr = TREE_CHAIN (attr))
{
+ tree selector = TREE_VALUE (TREE_VALUE (attr));
+ if (omp_context_selector_compare (ctx, selector) == 0)
+ {
+ error_at (loc,
+ "Multiple definitions of variants with the same "
+ "context selector violate the one-definition rule");
I think the following would be helpful (untested; needs nicher wording):
inform (DECL_SOURCE_LOCATION(TREE_PURPOSE (attr)), "... previous definition at ...."); Otherwise: I
think in C/C++ and, hence, in the middle end GCC uses lower case after
'error:' - contrary to Fortran that uses upper case after 'Error:'. * * *
Otherwise, LGTM.
Tobias
