[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

Mon Dec 9 11:11:45 PST 2019

jdoerfert added a comment.

> @jdoerfert , also, do we have tests that can go into the test suite / libomptarget regression tests demonstrating the collection of problems people have currently opened bugs on regarding math.h? I recall we still had problems with host code needing the long-double overloads, with constants from the system headers, etc.

The three tests I have in here show already that almost all of the known problems are solved by this (e.g. constants from the system headers). The rest can be easily added as lit test. The test suite situation is evolving but far from me being resolved. I would prefer not to mix these discussions and focus on lit tests with this patch (once split).

>> We restricted it for now to function definitions so we don't need to define the mangling as you cannot expect linking. (I did this to get it in TR8 while I figured it will solve all our math.h problems already).
>>  However, we need to avoid collisions with user code, e.g., through the use of symbols in the name that are not allowed to be used by the user (I thought "." is one of them).
> 
> Okay, but how to we distinguish functions for which there is a declaration and we need the mangling because the user has provided a definition elsewhere, from those for which there is a declaration, and we don't want mangling because we need to link to some system library?

The idea is, declarations inside begin/end declare variant are supposed to be not affected by the begin/end declare variant. That is, if you have declarations you cannot expect variant multi-versioning to happen. Having declarations inside or outside the begin/end declare variant is still fine if they all denote the same function.

> I don't think we should do this. Something similar to multiversioning is required only for a small subset.

This is neither true, nor relevant. It is not true because OpenMP 5.0 declare variant is so broken it cannot be used for what it was intended for. That means people (as for example we for math) will inevitably use begin/end declare variant.

> Everything else can be implemented in a more straightforward and simple way.

Having a single scheme is arguably simpler than maintaining multiple schemes. There is no additional overhead in using the more powerful and available multi-version scheme for everything.

>   Plus, I'm not sure that we'll need full reuse of the multiversioning. Seems to me, we can implement codegen in a different way. 

Please provide actual details with statements like this. It is impossible to tell what you mean.

> Multiversioning is supported only by x86 in clang/LLVM. I think we can try to implement a more portable and universal scheme.

This is not true, at least not from a conceptual standpoint. While cpu_supports and cpu_is multi-versioning is restricted to X86, see `supportsMultiVersioning` in TargetInfo.h,  the new kind of OpenMP multi-versioning is a portable and universal scheme (see the uses of `supportsMultiVersioning`)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71179/new/

https://reviews.llvm.org/D71179