Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A case for improved OpenMP support on CCI #24577

Open
valgur opened this issue Jul 10, 2024 · 8 comments
Open

A case for improved OpenMP support on CCI #24577

valgur opened this issue Jul 10, 2024 · 8 comments
Labels
question Further information is requested

Comments

@valgur
Copy link
Contributor

valgur commented Jul 10, 2024

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:

  • casadi: a symbolic framework for automatic differentiation and numeric optimization.
  • ceres-solver: large-scale, non-linear optimization library.
  • colmap: a photogrammetry software for 3D reconstruction from images.
  • ensmallen: a header-only C++ library for mathematical optimization.
  • fftw: a library for computing the discrete Fourier transform (DFT).
  • flann: a library for fast approximate nearest neighbors search in high-dimensional spaces.
  • imagemagick: a cross-platform software suite for displaying, converting, and editing raster images.
  • g2o: a framework for optimizing graph-based nonlinear error functions.
  • lightgbm: a gradient boosting framework that uses tree-based learning algorithms.
  • metis: a library for partitioning graphs, finite element meshes, and producing fill-reducing orderings.
  • opencv: an open-source computer vision and machine learning software library.
  • openmvg: an open-source multiple view geometry library for 3D reconstruction.
  • pcl: The Point Cloud Library for 2D/3D image and point cloud processing.
  • stdgpu: a library for efficient GPU computing with standard C++.
  • suitesparse: a suite of sparse matrix algorithms. The main open-source sparse matrix library.
  • vlfeat: an open-source library for computer vision applications.
  • xgboost: an optimized gradient boosting library designed for performance and speed.
  • xtensor: a C++ library for multi-dimensional arrays with NumPy-like syntax.

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.
  1. 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
  1. 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 #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 openmp/system: new recipe #22360. Except for the irrelevant linter errors, it works fine with the leftover llvm-openmp/18.1.8 from llvm-openmp: match FindOpenMP.cmake output, add MSVC support #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.

  1. 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 llvm-openmp: match FindOpenMP.cmake output, add MSVC support #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 valgur added the question Further information is requested label Jul 10, 2024
@valgur
Copy link
Contributor Author

valgur commented Jul 10, 2024

@jcar87

@valgur
Copy link
Contributor Author

valgur commented Jul 10, 2024

The proposed solution is basically

but for CCI.

@AbrilRBS
Copy link
Member

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
Copy link
Contributor Author

valgur commented Jul 11, 2024

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
Copy link
Contributor

jcar87 commented Jul 11, 2024

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:

  • static libraries would have to express the runtime as a system dependency - this is no different than what happens with the pthreads library on Linux, or some other common ones (like ws2_32 on Windows). Some libraries already have this logic, and it should work.
    • For shared libraries everything should already work, without the need for anything extra in package_info.
    • Alternatively, for non-msvc compilers, if the compiler is invoked to link, the opemp flag can be passed as well and will cause the runtime to be linked (this can help abstracting away the naming of the runtime library).
  • for libraries that have public openmp usage in headers, the relevant compiler flags need to be propagated. But my understanding is that there is a very small number of recipes affected by this:
    • lightgbm
    • mlpack
    • stdgpu
    • ensmallen
  • the apple clang compiler can process the pragmas (by forwarding a flag), but does not have the headers or the runtime. Here is where the llvm-openmp package could help - but at the same time, users know that apple-clang does not support OpenMP, so I don't think they have an expectation that conan center will change the reality.

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

  • we want a package to abstract the flags for recipes that need to propagate them, and we want to avoid a 20-something line logic in 4 recipes and change it with 2 lines in those recipes, and a full-fledged recipe. This is a worthy goal, but it needs to be clarified that we are not blocked at all. A well motivated PR for each of those 4 recipes will be considered with proposed logic.
  • we may want a more uniform way of expressing the runtime to support static libraries (some already do this, but it's not done in a consistent way)
  • we want to provide the LLVM runtime for apple-clang, because apple-clang does not come with one

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:

  • homebrew llvm package
  • clang binaries provided by llvm
  • clang-cl provided by visual studio installer

@valgur
Copy link
Contributor Author

valgur commented Jul 11, 2024

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
Copy link
Contributor Author

valgur commented Jul 17, 2024

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
Copy link
Contributor Author

valgur commented Jul 20, 2024

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:

  • acado
  • amgcl
  • armadillo
  • base64
  • bimg
  • blaze
  • blis
  • boost
  • bullet3
  • c-blosc2
  • casadi
  • celero
  • cern-root
  • cgal
  • cilantro
  • cimg
  • colmap
  • cryptopp
  • darknet
  • detools
  • eigen
  • fftw
  • freeimage
  • g2o
  • gdal
  • gettext
  • ginkgo
  • gklib
  • glim
  • gnulib
  • gst-plugins-bad
  • gtsam
  • gtsam_points
  • highs
  • imagemagick
  • ktx
  • libaom-av1
  • libarchive
  • libbasisu
  • libb2
  • libcvd
  • libelas
  • libigl
  • libinterpolate
  • libnabo
  • libraw
  • librealsense
  • libsvm
  • libvpx
  • libxcrypt
  • libyuv
  • lightgbm
  • meshoptimizer
  • metall
  • metis
  • mlpack
  • msdfgen
  • muparser
  • nanort
  • ncbi-cxx-toolkit-public
  • netpbm
  • nmslib
  • octomap
  • ogdf
  • ogre
  • onedpl
  • onnxruntime
  • openblas
  • opencv
  • openmpi
  • openmvg
  • opensubdiv
  • openvino
  • ouster_sdk
  • pcl
  • pdal
  • pgm-index
  • pixman
  • procxx-boost-ext-simd
  • qt
  • quantlib
  • quickcpplib
  • rvo2
  • sail
  • samurai
  • scip
  • seqan
  • seqan3
  • simde
  • sleef
  • small_gicp
  • soundtouch
  • soxr
  • statslib
  • stdgpu
  • stella-cv-fbow
  • stella_vslam
  • suitesparse-config
  • suitesparse-cholmod
  • suitesparse-graphblas
  • suitesparse-lagraph
  • suitesparse-paru
  • suitesparse-spex
  • sundials
  • symengine
  • tensorflow-lite
  • tesseract
  • thrust
  • tiledb
  • tiny-dnn
  • tinyexr
  • tlx
  • tracy
  • vcglib
  • vigra
  • vlfeat
  • vvenc
  • xgboost
  • xtensor
  • xxsds-sdsl-lite
  • zfp
  • zstd

@jcar87

phbasler added a commit to phbasler/conan-center-index that referenced this issue Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants