search

conan-io / conan-center-index

Recipes for the ConanCenter repository
https://conan.io/center
MIT License
942 stars 1.7k forks source link

A case for improved OpenMP support on CCI #24577

Open valgur opened 1 month ago

valgur commented 1 month ago

Motivation

OpenMP (Open Multi-Processing) is a widely adopted API that provides parallel programming in C, C++, and Fortran, optionally with multi-platform shared-memory support. It is a critical building block for many numerical and scientific libraries, but is used in general-purpose software as well for easy parallelization support.

OpenMP consists of a set of #pragma omp ... compiler directives that get translated into OpenMP API calls, which are then handled by the runtime library, which is often (but not always) provided with the compiler toolchain and linked dynamically. The translation of OpenMP directives needs to be enabled with a suitable compiler flag, typically -fopenmp or /openmp. Passing the same flag to a linker instructs it to link against the OpenMP runtime library found in the compiler toolchain.

Based on the info in recipes alone, there are currently about 50 recipes on CCI currently or upcoming in PRs. Some more notable ones include:

Although some of these libraries support alternative parallelization mechanisms, such as plain pthreads or TBB, several of these libraries don't and are entirely (e.g. SuiteSparse) or close to unusable without parallelization via OpenMP enabled.

Problems

  1. OpenMP runtime libraries are missing from Clang and AppleClang. From both the CI and by default from consumer systems as well.
  2. Using OpenMP as a dependency in recipes needs a lot of extra logic. This is both cumbersome and bug-prone. A typical setup looks like (varying a bit based on whether compiler directives are used in public headers or not): 
    
def requirements(self):
if self.options.with_openmp and self.settings.compiler in ["clang", "apple-clang"]:
    self.requires("llvm-openmp/17.0.6", transitive_headers=True, transitive_libs=True)

@property def _openmp_flags(self): if self.settings.compiler == "clang": return ["-fopenmp=libomp"] elif self.settings.compiler == "apple-clang": return ["-Xclang", "-fopenmp"] elif self.settings.compiler == "gcc": return ["-fopenmp"] elif self.settings.compiler == "intel-cc": return ["-Qopenmp"] elif self.settings.compiler == "sun-cc": return ["-xopenmp"] elif is_msvc(self): return ["-openmp"] return None

def package_info(self): ... if self.options.with_openmp: if self.settings.compiler in ["clang", "apple-clang"]: self.cpp_info.requires.append("llvm-openmp::llvm-openmp") openmp_flags = self._openmp_flags self.cpp_info.cflags = openmp_flags self.cpp_info.cxxflags = openmp_flags self.cpp_info.sharedlinkflags = openmp_flags self.cpp_info.exelinkflags = openmp_flags

3. 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](https://cmake.org/cmake/help/latest/module/FindOpenMP.html).
   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 #22353 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.

# Solutions

1. Create an `openmp` meta-package - add `llvm-openmp` as a requirement for Clang and AppleClang and export the appropriate compiler and linker flags for all others. I have an experimental version available in #22360. Except for the irrelevant linter errors, it works fine with the leftover `llvm-openmp/18.1.8` from #22353. There is not much of a precedent for meta-packages on CCI and Conan does not have a suitable `package_type` for it, but it's something that can hopefully applied for other packages with many alternative implementations in the future as well, such as `blas`, `lapack` and maybe even `jpeg`. The benefits of having a separate `openmp` package are:
- The exporting of appropriate build flags without the redundancy and potential bugs.
- Maps well from the `OpenMP` dependency in CMake when writing a recipe.
- Allows `transitive_headers=True` and `transitive_libs=True` to clearly state the the library uses OpenMP directives in its public headers.
- Allows consumers to swap out or customize the `openmp` package for an alternative runtime or some less common compiler support.
- The `llvm-openmp` package can be pinned to the latest available version for each major version matching the compiler / LLVM version. That would guarantee the stability of ABI and API of the library dependency, to be safe. Realistically, I would expect the latest version to be backwards- and forwards-compatible (up to the available OpenMP standard standard supported by it, of course) thanks to the highly standardized API, plus some non-standard but stable extensions by GOMP, for example. Since `libomp` and others don't apply SONAME versioning to their library files, ABI compatibility can most likely be assumed as well. I have not looked into the compatibility guarantees any closer than that, though.

  Alternatively, the C3I Docker images could simply be updated to include `libomp-dev` or the equivalent, but that would not fix the missing library issues for consumers and there's no real blockers to using the `llvm-openmp` package (unlike for CUDA, for example), as far as I can tell. 

2. Fix the `FindOpenMP.cmake` compatibility issues in `llvm-openmp`. There are basically two options:
  - 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.
  - 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 #22353.

I'm fine with either approach, 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` if the consumer has a specific reason to prefer it and knows what they are doing.

That's all.
valgur commented 1 month ago

@jcar87

valgur commented 1 month ago

The proposed solution is basically

but for CCI.

AbrilRBS commented 1 month ago

Hi @valgur thanks a lot for the write-up, we appreciate it :)

I just wanted to let you know that we are in fact looking into this and how to best approach openmp support - it might even include some built-in Conan client support, but we're in preliminary stages yet! Thanks a lot for your effort and patience while we also figure this out

valgur commented 1 month ago

Ok, I'm glad to hear that. Thanks!

Realistically, though, the OpenMP runtime and the preprocessor flag will probably need to be included in the dependency graph in one form or another, with the correct visibility as well, right? If not as a (meta-)package, then do you have some kind of an alternative mechanism in mind, perhaps? Not intending this as a critique, just curious. If it's going to end up looking more or less like a package anyway, then could you still consider the openmp meta-package approach in the mean time? The approach looks very viable to me and only basically does the same that is already done in existing recipes, just less sloppily. This would allow OpenMP usage to be cleaned up in recipes that depend on it and once there's a more appropriate solution available, then maybe:

  1. deprecate the openmp package,
  2. replace the internals of the openmp package with the alternative solution to migrate without breaking things,
  3. update the packages depending on openmp at a steady pace with whatever the alternative solution is, if necessary.

The llvm-openmp issues should still be addressed, though, imo, so I'll reopen the PR and apply any tweaks that you deem necessary.

jcar87 commented 1 month ago

Still working on this - however, I think it needs to be clarified that there is no limitation in Conan Center recipes to support OpenMP, or with Conan in general.

Recipes that currently support OpenMP should already work for the most part, provided the compiler supports it and it comes with the appropriate runtime.

There are a few limitations currently:

so If I'm understanding correctly, the real issues are:

Not included in major Linux distributions with the clang package by default (Ubuntu, Arch, Fedora)

This is a worthy discussion with the distro maintainers too. The matching libomp headers and runtime are made available by the distros. On ubuntu libomp-dev package needs to be installed. I would suggest opening an issue with Ubuntu, or debian if a default installation of the clang package does not have a functional -fopenmp, it may be argued this is a bug, for example https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882781. I can see the llvm-toolchain package is more complete, but unsure why that is not part of the default debian installations.

please note that the assessment of the debian maintainers:

openmp is a niche and I don't think we should have a hard dependency for every clang user.

seems to contradict the "widely adopted" premise in this issue. For what is worth, their assessment matches what we have seen - we don't have any issues reported in Conan for openmp support, and in Conan Center we've never had requests or issues reported by users of the libraries, all openmp related discussions seem to be motivated by recipe maintainers. I should remind that our focus should always be the users - and that our efforts and priorities are guided by demand from the users of recipes. We don't want to run the risk of diverting resources for something that may not actually be used.

Please note that these distributions of clang do contain the runtime, so we can't generalise around compiler=clang:

valgur commented 1 month ago

The fact that we are discussing the problems and correct ways of using the library should be enough of a reason for encapsulating and abstracting away the complexity, especially if there's a straightforward and low-risk solution available (based on what has been discussed so far at least). While a motivated developer can figure it out on their own, sure, it does not mean that they should.

Regarding the interest from "real users" - the near complete lack of computer vision, ML, and robotics communities on Conan/CCI does not mean that they don't exist or care. Far from it. See some of the most actively discussed packages on Vcpkg: colmap, ceres, suitesparse, lapack. And while it's much more down to CUDA support, other technical reasons or plain momentum, having to fight Conan where Vcpkg gets by with a nice abstraction with proper visibility support from CMake out of the box really does not help move the needle in the right direction.

valgur commented 1 month ago

Regarding whether splitting libomp-dev into an optional package from clang/llvm packages in Linux distros is correct or not, that's business as usual for them and hardly unique to libomp. Installing extra packages for libraries for a build is the norm and expected. There's also the quite niche libc++ header/runtime library that they don't include with their clang package by default on Debian/Ubuntu, for example.

valgur commented 1 month ago

To give some more objective numbers, I grepped through all of the source archives of the latest versions of packages on CCI (plus some pending PRs) for #pragma omp and #include <omp.h>. There are currently 122 packages that use OpenMP directly, of which 76 use it in a header file (not all of which are exposed publicly, of course).

For reference, that's more than double the number of direct uses of libjpeg (56), libpng (56), xorg/system (50) or opengl/system (38), but less than zlib (175) and pthread (427).

Here's a detailed list including all of the packages and their respective files (excluding tests, examples and vendored libraries unvendored by the recipes) that make use of OpenMP: https://gist.github.com/valgur/15d5dc0c31dbfc94f168188a4d1859f7

And just the list of packages, where usages in headers are in bold:

@jcar87