This is the second part loosely extracted from D71179 and cleaned up. This patch provides semantic analysis support for `omp begin/end declare variant`, mostly as defined in OpenMP technical report 8 (TR8) . The sema handling makes code generation obsolete as we generate "the right" calls that can just be handled as usual. This handling also applies to the existing, albeit problematic, `omp declare variant support`. As a consequence a lot of unneeded code generation and complexity is removed. A major purpose of this patch is to provide proper `math.h`/`cmath` support for OpenMP target offloading. See PR42061, PR42798, PR42799. The current code was developed with this feature in mind, see . The logic is as follows: If we have seen a `#pragma omp begin declare variant match(<SELECTOR>)` but not the corresponding `end declare variant`, and we find a function definition we will: 1) Create a function declaration for the definition we were about to generate. 2) Create a function definition but with a mangled name (according to `<SELECTOR>`). 3) Annotate the declaration with the `OMPDeclareVariantAttr`, the same one used already for `omp declare variant`, using and the mangled function definition as specialization for the context defined by `<SELECTOR>`. When a call is created we inspect it. If the target has an `OMPDeclareVariantAttr` attribute we try to specialize the call. To this end, all variants are checked, the best applicable one is picked and a new call to the specialization is created. The new call is used instead of the original one to the base function. To keep the AST printing and tooling possible we utilize the PseudoObjectExpr. The original call is the syntactic expression, the specialized call is the semantic expression.
We're not extremely consistent about how we phrase these kinds of errors, but I grepped for stray and we have only one message about stray tokens, but grepped for matching and we have a bunch more. Phrasing this as:
'#pragma omp end declare variant' with no matching ''#pragma omp begin declare variant'
might be better.
Please add a comment here explaining why the TemplateParameterLists.empty() check is here.
Is there any way in which this name might become visible to users (e.g., in error messages)?
Please provide an example or otherwise explain why this failure behavior is correct.
Why is the ordering here useful? Don't you collect all of the variant clauses from all of the declarations? Can't there be duplicates? (and does the relative order need always be the same?) Are we effectively supporting here, as an extension, cases where not all of the declarations have the same set of variants declared (it loos like we are because there's no break in the while (CalleeFnDecl) loop, but this makes me wonder if that would still have an odd behavior.
Yes, I think so. One way to trigger it would be to define the same function in the same omp begin declare variant scope (or two with the same context). I haven't verified this though.
Tried to improve the comment.
I don't think it is ordered. We have a conceptual set and BestIdx determines the which of the set is the best. All others are equal.
Yes (and no). getBestVariantMatchForContext should determine the best regardless of duplicates, we might just try it multiple times if we didn't manage to create a call.
We are supporting that. All declarations are scanned and all variants are collected. What odd behavior do you refer to?
I think that we have to consider diagnostic quality as a first-class citizen. There are a few different ways that we might approach this. A minimal diff from what you have might be:
(I think that (1) is a good idea anyway).
Another way would be to add some first-class property to the function and then leave the mangling to CodeGen. Are we mangling these in Sema in other places too?
Also, these can have external linkage, right?
Makes sense, thanks.
hide the mangling consistently from the user (and fix rebase bug)
I added a test for this in begin_declare_variant_messages.c and confirmed my earlier statement. Then I implemented what you describe above (in spirit).
That would potentially work but require us to touch a lot of places that deal with redefinitions and overload resolution. Getting the right clashes between specialized versions with the same name and openmp context but not getting the clashes for the same name but different openmp context will be non-trivial. Similarly, we need to ignore specialized versions during overload resolution, etc. etc.
FWIW, this is design 8 or 9, or even more. I tried to keep the names till codegen in the beginning, using an approach similar to what you described. I tried to use namespaces (even in C) to get the right kind of name clashes and interactions, I tried ... This is the only thing that (so far) worked reliably and is (IMHO) very non-intrusive.
Yes. My name clash test has 3 different linkages, see begin_declare_variant_messages.c.