llvm-openmp: fix incompatibilities with official FindOpenMP, add v18.1.8, simplify patches, add MSVC support

[valgur]

Summary

Changes to recipe: llvm-openmp/*

Motivation

See #24577 for more context. Re-opens #22353.

The llvm-openmp recipe exports a FindOpenMP.cmake module with the appropriate OpenMP::OpenMP_CXX and OpenMP::OpenMP_C targets, but is otherwise not compatible with the output variables and version set by the official FindOpenMP.cmake module.
Here's the current output for find_package(OpenMP) with llvm-openmp:

-- Conan: Target declared 'OpenMP::OpenMP'
OpenMP_FOUND: 
OpenMP_VERSION: 17.0.6
OpenMP_C_FOUND: 
OpenMP_CXX_FOUND: 
OpenMP_CXX_VERSION: 
OpenMP_CXX_SPEC_DATE: 
OpenMP_CXX_INCLUDE_DIRS: 
OpenMP_CXX_LIB_NAMES: 
OpenMP_CXX_LIBRARIES: 
OpenMP_CXX_FLAGS: 
OpenMP_omp_LIBRARY: 

Here's the output with the fixes addressing this issue from this PR applied:

-- Conan: Component target declared 'OpenMP::OpenMP'
-- Conan: Target declared 'llvm-openmp::llvm-openmp'
-- Conan: Including build module from '/home/martin/.conan2/p/b/llvm-e49ba89a6637a/p/lib/cmake/openmp/conan-llvm-openmp-vars.cmake'
-- Found OpenMP: -fopenmp=libomp (found version "5.0")
OpenMP_FOUND: TRUE
OpenMP_VERSION: 5.0
OpenMP_C_FOUND: TRUE
OpenMP_CXX_FOUND: TRUE
OpenMP_CXX_VERSION: 5.0
OpenMP_CXX_SPEC_DATE: 201611
OpenMP_CXX_INCLUDE_DIRS: /home/martin/.conan2/p/b/llvm-e49ba89a6637a/p/include
OpenMP_CXX_LIB_NAMES: omp;m;dl;pthread;rt
OpenMP_CXX_LIBRARIES: llvm-openmp::llvm-openmp
OpenMP_CXX_FLAGS: -fopenmp=libomp
OpenMP_omp_LIBRARY: llvm-openmp::llvm-openmp

Many (typically older) projects rely on these variables being available. The official module also supports C, CXX and Fortran components in the find_package() call, which the exported module ignores.

Details

Possible solutions to the incompatibilities are:

• Disable all compilers except for Clang and AppleClang in validate() and create a thin wrapper around CMake's FindOpenMP.cmake. This only works for Clang/AppleClang because CMake's FindOpenMP.cmake always selects the compiler's native OpenMP runtime to link against and would lead to mixing of libomp and the native runtime in transitive dependencies, which is not safe. Note that CMake's FindOpenMP.cmake assumes that the runtime library is on the compiler's default search paths, so creating such a wrapper might not be as simple as it looks at first glance and the only real benefit ends up being not including the OpenMP version detection logic in the wrapper directly. Getting the external try_compile() to work reliably is tricky, though, and the picked OpenMP flags might accidentally differ from the ones propagated by package_info(), so I'm not a fan of this approach.
• Add a CMake module that detects the OpenMP standard supported by the compiler preprocessor and exports the necessary CMake variables. This will duplicate CMake's FindOpenMP somewhat, but not by a significant amount since it does not need to be nearly as generic. This will work as expected with all compilers - use the compiler's preprocessor for the translation and libomp as the runtime. The FindOpenMP interface has been very stable as well and can easily be updated if it does changes (as it did in 3.30). This is the approach that was proposed in llvm-openmp: match FindOpenMP.cmake output, add MSVC support #22353.

I'm fine with either approach, ultimately, but lean towards the latter. Not because I think that other compilers should use libomp for whatever the reason, but rather because I don't see a reason to limit the use of the package to provide libomp unnecessarily if the consumer has a specific reason to prefer it and knows what they are doing.

I have tested this recipe with tens of different packages on my private fork of CCI: https://github.com/valgur/conan-center-index
The only real failure case is when an external project does just compile_options(${OpenMP_CFLAGS}) or similar, since passing only -fopenmp to the compiler without adding the includes and runtime linking is not sufficient. Unfortunately, there is no good fix for this unless I add an ugly switch for all possible compilers and add the equivalent of -I... -L... -lomp to the flags. Patching such projects to use link_libraries(OpenMP::OpenMP_C) and submitting the patch upstream as well is probably a much better way to handle this.

TODO:

• Revert the renaming of libomp.dll -> omp.dll.
• Add copyright info about the parts of the CMake module copied from the CMake's official implementation.
• Change the default to shared=True.
• Mention that OpenMP_RUNTIME_MSVC is intentionally not applied. Maybe raise an error when it's set to a value other than llvm.

---

• Read the contributing guidelines
• Checked that this PR is not a duplicate: list of PRs by recipe
• Tested locally with at least one configuration using a recent version of Conan

[valgur]

I think I'm going to revert the libomp.dll->omp.dll renaming on MSVC, but it's going to need some testing because the default build had some issues when built with Conan (llvm/llvm-project#88876).

[valgur]

To address the comments by @jcar87 at #22353 (comment):

I think most of the comments miss the mark a bit and approach it from the vantage point of trying to use llvm-openmp as the sole OpenMP runtime on CCI. That was definitely not my original intention, which is described in #24577 and #22360. Rather, I kind of gave up on the original idea after a half a year of near silence on the topic, not wanting to spend the next several months on waiting on something that's not really of interest to the maintainers. So I opted for the less than ideal solution that works close enough, but allows moving forward with (imo) important missing packages such as #5763 (comment) and #23744 (comment). I guess the gratingly bad approach at least got the discussion started, but I'm not proud of going there. Sorry about that.

I'll be skipping the points that I already covered in the issue description. Correct me if I miss anything.

In general, all OpenMP runtimes implement the same API and some of the stable extensions of each other (e.g. from GOMP), so they are intended to be mutually compatible. While that does not seem reliable enough to be used as the general default, which I agree with, I don't see a reason to disable this choice for users who are aware of what they are doing from using the libomp runtime with other compilers.

Furthermore, other repository maintainers have explicitly avoided making the LLVM omp.h headers visible to gcc:

• libomp header file conflicts with gcc openmp compilation Homebrew/homebrew-core#112107
• https://bugzilla.redhat.com/show_bug.cgi?id=1451138

Unless a Conan user goes out of their way to use Conan in a more exotic fashion, Conan packages are isolated and the include dirs are not placed on a globally visible path that might trip up GCC, unless the user is explicitly linking against the targets in this package.

For gcc, the -fopenmp flag implies -lgomp , which will be passed to the linker regardless. So there is a very real risk of passing both -lomp and -lgomp. Depending on some circumstances (the build system used, shared/private, the link order, whether the linker has —as-needed or not), the following could happen:

• Both omp (from this package) and gomp from the default installation are visible to the linker, in which case the linker may decide one or the other.
• If for whatever reason both libomp and libgomp are to be loaded at runtime (for example, in those cases where -as-needed is NOT passed by the linker), this will cause issues : https://cpufun.substack.com/p/is-mixing-openmp-runtimes-safe . These could be: missing symbols (because gcc expects some internal symbols in libgomp), and in the worst case, the executable loads both libraries but does not work (does not actually parallelise anything)

While I can't say if this is dependent on the order of the build flags, as the test_package demonstrates and after using this recipe for tens of packages with GCC, the only time libgomp.so was present as NEEDED in the shared libraries was when a project was explicitly adding -lgomp for GCC.

The user will have to take care not to accidentally link against external or system libraries that link against libgomp, no doubt. I can add a warning when using the package with non-Clang/AppleClang/MSVC compilers, perhaps. The project also includes an option to create symlinks aliases for libgomp and libiomp5 from libomp, which could maybe be enabled to further avoid that case.

For Visual Studio/msvc, using /openmp:llvm is still described as experimental
* https://learn.microsoft.com/en-us/cpp/build/reference/openmp-enable-openmp-2-0-support?view=msvc-170

• https://devblogs.microsoft.com/cppblog/improved-openmp-support-for-cpp-in-visual-studio/

With no indication that this is already available for production code. Additionally, it would seem to be the case that The libomp runtime is already shipped by visual studio, is it not usable? Why would one need to use a newer runtime, does it not work?

Again, if the user is specifically using llvm-openmp with MSVC, then forcing /openmp:llvm is the correct thing to do. Since package_info() would export that flag anyway, the opposite would lead to mixing runtimes.

There’s also loss of functionality in the file provided in this PR:

• A user may expect to be able to influence which variant of the OpenMP flag is used by mcvc (with OpenMP_RUNTIME_MSVC which was added in CMake 3.30), but then find that conan is overriding this for them and making a different choice altogether.

I intentionally disregarded that option for the reason mentioned above. I could add an explicit comment in the wrapper about that, though.

• There is logic that appears to have been "borrowed" from FindOpenMP.cmake in this PR - it needs to be fully identified, and we need to evaluate whether we need to propagate any license disclaimers.

I'll take care of that, sure. The only copied parts are the detection of the detection of the OpenMP version supported by the preprocessor and mapping that to spec date. The rest is original.

The motivation this PR description would be insufficient, in the sense that what is happening in this recipe, is not representative of the most common case: to have OpenMP directives in a public header, causing compiler flags to propagate. I doubt this is the most common case - what could happen however is that when libraries are built statically, one must propagate the relevant runtime as a system library (I see some recipes propagating -fopenmp but this would be unnecessary in most cases)

I mean... you answered that yourself - it's definitely needed for static libraries. You will either need to pass -fopenmp or similar to the linker or add the appropriate runtime to system_libs. It's not much of a difference and the prior is slightly simpler.
There are quite a few libraries that do use #pragma omp ... in their public headers, though, which need the compiler flags as well. See https://github.com/search?q=repo%3Avalgur%2Fconan-center-index+self.requires%28%22openmp%2Fsystem%22%2C+transitive_headers%3DTrue%2C+transitive_libs%3DTrue%29&type=code

To summarise the concerns - we are outside of the comfort zone and it feels like are propagating this requirement (on this recipe) across conan center, is experimenting a tad too much

I agree and understand. I'm definitely open to a better solution.

• the shared=False default - why it aligns with Conan Center, it's also unusual to use a static OpenMP runtime.

That's a very good point. I'll change the default to shared=True.

[valgur]

While I'm still not advocating for using LLVM OpenMP in place of GOMP or other native OpenMP runtimes, an interesting data point in that regard is conda-forge.

While the main Anaconda channel uses gomp as its OpenMP implementation (https://anaconda.org/anaconda/_openmp_mutex/files), conda-forge adds support for llvm-openmp (https://github.com/conda-forge/openmp-feedstock/blob/main/recipe/meta.yaml#L27-L39) and simply symlinks libgomp.so to libomp.so in the process: https://github.com/conda-forge/_openmp_mutex-feedstock/blob/main/recipe/build.sh#L12

The LLVM variant of _openmp_mutex has been downloaded a total of 9 million times, llvm-openmp 15 million times and _openmp_mutex in general across all channels about 222 million times in total.

The symlinking of libgomp to libomp does not appear to have lead to any reported issues as far as I can tell:

• https://github.com/conda-forge/_openmp_mutex-feedstock/issues?q=
• https://github.com/search?q=org%3Aconda-forge+openmp&type=issues
• https://github.com/search?q=org%3Aconda-forge+_openmp_mutex&type=issues
• https://github.com/search?q=libomp+libgomp&type=issues

[valgur]

This is ugly, but a potential alternative of passing the appropriate -I/-L paths together with -fopenmp did not work due to the -L flag not getting propagated to the linker when passed via CMAKE_C_FLAGS, which is how this is typically used.

Fortunately, most modern CMake projects use the OpenMP targets instead and the rest can be patched quite easily. Here's a list of such projects (ticked ones have a patch available): https://gist.github.com/valgur/cbd1a856a2eb42c285a2360ecceb0405

The only project that truly needed the flags variable instead of CMake targets was VTKm: https://gitlab.kitware.com/vtk/vtk-m/-/blob/master/CMake/VTKmDeviceAdapters.cmake#L64

[conan-center-bot]

Conan v1 pipeline ✔️

All green in build 13 (6d2542a525966c9a4f5bcb830eb4189023d5bdfa):

• llvm-openmp/18.1.8:
  Built 16 packages out of 22 (All logs)

• llvm-openmp/17.0.4:
  Built 16 packages out of 22 (All logs)

• llvm-openmp/17.0.6:
  Built 16 packages out of 22 (All logs)

• llvm-openmp/14.0.6:
  Built 16 packages out of 22 (All logs)

• llvm-openmp/16.0.6:
  Built 16 packages out of 22 (All logs)

• llvm-openmp/12.0.1:
  Built 16 packages out of 22 (All logs)

• llvm-openmp/11.1.0:
  Built 18 packages out of 22 (All logs)

• llvm-openmp/13.0.1:
  Built 16 packages out of 22 (All logs)

• llvm-openmp/15.0.7:
  Built 16 packages out of 22 (All logs)

---

Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 13 (6d2542a525966c9a4f5bcb830eb4189023d5bdfa):

• llvm-openmp/15.0.7:
  Built 6 packages out of 10 (All logs)

• llvm-openmp/18.1.8:
  Built 8 packages out of 10 (All logs)

• llvm-openmp/17.0.6:
  Built 8 packages out of 10 (All logs)

• llvm-openmp/17.0.4:
  Built 8 packages out of 10 (All logs)

• llvm-openmp/14.0.6:
  Built 6 packages out of 10 (All logs)

• llvm-openmp/16.0.6:
  Built 6 packages out of 10 (All logs)

• llvm-openmp/12.0.1:
  Built 6 packages out of 10 (All logs)

• llvm-openmp/13.0.1:
  Built 6 packages out of 10 (All logs)

• llvm-openmp/11.1.0:
  Built 6 packages out of 10 (All logs)