Skip to content

Conversation

@perseoGI
Copy link
Contributor

Summary

Changes to recipe: fbgemm/1.3.0

Motivation

As part of the devendorization process of libtorch dependencies, this recipe was not yet included in CCI and it is already present in several distributions, see repology.

Details

Straightforward library:

  • C and C++ compatible
  • Only supported in architectures with AVX512 instruction set, so ARM and x86 are excluded
  • It requires some CMake patches in order to unvendor cpuinfo and asmjit (both with transitive_headers and libs enabled as they are used in the public headers)
    • Find libraries
    • Link libraries against the generated targets
    • Remove fixed C++ and C standards and avoid -Werror
    • Avoid installing vendored dependencies (last part of the patch)
  • The official CMake file name of the library is: fbgemmLibrary and the official CMake target is fbgemm
  • This library defaults links against OpenMP so pertinent compiler flags should be added in gcc and clang as msvc defaults link against it

  • Read the contributing guidelines
  • Checked that this PR is not a duplicate: list of PRs by recipe
  • If this is a bug fix, please link related issue or provide bug details
  • Tested locally with at least one configuration using a recent version of Conan

Add a 👍 reaction to pull requests you find important to help the team prioritize, thanks!

check_min_cppstd(self, 17)
if self.settings.get_safe("compiler.cstd"):
check_min_cstd(self, 99)
if "arm" in self.settings.arch or self.settings.arch == "x86":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is true? I can see conditionals for arm in the library's CMakeLists.txt: https://github.com/pytorch/FBGEMM/blob/c93ec6e3b57f01ffd94335cef11c807430a836ca/CMakeLists.txt#L166

Comment on lines +49 to +50
if "arm" in self.settings.arch:
raise ConanInvalidConfiguration("fbgemm is not supported on ARM")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this restriction

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants