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

LLVM 13 and 14 #7613

Closed
wants to merge 48 commits into from
Closed

LLVM 13 and 14 #7613

wants to merge 48 commits into from

Conversation

rdeterre
Copy link
Contributor

@rdeterre rdeterre commented Oct 8, 2021

Specify library name and version: llvm/13.0.0

This is a new PR for the changes from @Manu343726 and @tcanabrava to add an LLVM recipe.

@CLAassistant
Copy link

CLAassistant commented Oct 8, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 4 committers have signed the CLA.

❌ alazartech
❌ Manu343726
❌ tcanabrava
❌ rdeterre
You have signed the CLA already but the status is still pending? Let us recheck it.

@ghost
Copy link

ghost commented Oct 8, 2021

I detected other pull requests that are modifying llvm/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@rdeterre rdeterre changed the title LLVM 13.0.0 LLVM 13 and 14 Apr 20, 2022
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Failure in build 12 (07599464c76a526749d1d9893bb0e03c31328711):

An unexpected error happened and has been reported. Help is on its way! 🏇

@jusito
Copy link

jusito commented Apr 28, 2022

I took a look at job pool calculation, iam still unsure if this is solid enough to add it, but I want to at least share it because I brought it up.

  1. Noting LLVM_PARALLEL_COMPILE_JOBS was quite helpful, assuming we know how much ram per compile / link job is consumed.
    • We could calculate it in python but this needs another dependency for cross platform
    • We could patch HandleLLVMOptions.cmake to calculate it from within cmake (patch below)
  2. One issue is that link and compile jobs can run at the same time and that there is no way to say "this job pool is exclusivly" or a ninja option for available memory like "-l x" for cpu.
    • Logged the process history and used clang-12 + ld (not lld), seems like max(link job ram) + 2*max(compile job ram) should be enough
    • used: cmake -G Ninja -S llvm -B build -DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra;libc;libclc;lld;lldb;openmp;polly;pstl;compiler-rt;libc;openmp;' -DLLVM_ENABLE_RUNTIMES='libcxx;libcxxabi;libunwind;'
  3. another issue is that there are only certain jobs which are needing lots of memory, libLLVM.so, clang-tidy, ... so actually most optimized way would be to have individual pools per target, or maybe run it multiple times with different targets/config. Which I dont think is a good idea to add here.
  4. The resource usage and runtime, and therefore overlapping resource usage, can change a lot according to used compiler, linker, architecture, ninja version e.g. the new heuristics for ninja in pr 2019 also explained in ninja issue 1335.

The easiest way is therefore to set LLVM_PARALLEL_LINK_JOBS=1, not using ninja or limit ninja jobs.
A more optimized solution is this patch, giving it a high value for LLVM_RAM_PER_LINK_JOB will have the same effect on system with less memory but allows greater machine to use some of the available resources. Some compile jobs take about 2gb, link about 10gb and the spike is around 1 link + 2 compile jobs => 14gb for link to ensure there is enough ram during both. To be safe maybe add a bit more, just to be stable.

diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index fcaa8f20bf94..00c6fbf6fa8e 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -31,8 +31,25 @@ string(TOUPPER "${LLVM_ENABLE_LTO}" uppercase_LLVM_ENABLE_LTO)
 
 # Ninja Job Pool support
 # The following only works with the Ninja generator in CMake >= 3.0.
+if (NOT LLVM_RAM_PER_COMPILE_JOB)
+  set(LLVM_RAM_PER_COMPILE_JOB "2000")
+endif()
 set(LLVM_PARALLEL_COMPILE_JOBS "" CACHE STRING
   "Define the maximum number of concurrent compilation jobs (Ninja only).")
+
+cmake_host_system_information(RESULT AVAILABLE_PHYSICAL_MEMORY QUERY AVAILABLE_PHYSICAL_MEMORY)
+cmake_host_system_information(RESULT NUMBER_OF_LOGICAL_CORES QUERY NUMBER_OF_LOGICAL_CORES)
+if(LLVM_RAM_PER_COMPILE_JOB)
+  math(EXPR memory_available_jobs "${AVAILABLE_PHYSICAL_MEMORY} / ${LLVM_RAM_PER_COMPILE_JOB}" OUTPUT_FORMAT DECIMAL)
+  if (memory_available_jobs LESS 1)
+    set(memory_available_jobs 1)
+  endif()
+  if (memory_available_jobs LESS NUMBER_OF_LOGICAL_CORES)
+    set(LLVM_PARALLEL_COMPILE_JOBS "${memory_available_jobs}")
+  else()
+    set(LLVM_PARALLEL_COMPILE_JOBS "${NUMBER_OF_LOGICAL_CORES}")
+  endif()
+endif()
 if(LLVM_PARALLEL_COMPILE_JOBS)
   if(NOT CMAKE_GENERATOR MATCHES "Ninja")
     message(WARNING "Job pooling is only available with Ninja generators.")
@@ -42,8 +59,22 @@ if(LLVM_PARALLEL_COMPILE_JOBS)
   endif()
 endif()
 
+if (NOT LLVM_RAM_PER_LINK_JOB)
+  set(LLVM_RAM_PER_LINK_JOB "14000")
+endif()
 set(LLVM_PARALLEL_LINK_JOBS "" CACHE STRING
   "Define the maximum number of concurrent link jobs (Ninja only).")
+if(LLVM_RAM_PER_LINK_JOB)
+  math(EXPR memory_available_jobs "${AVAILABLE_PHYSICAL_MEMORY} / ${LLVM_RAM_PER_LINK_JOB}" OUTPUT_FORMAT DECIMAL)
+  if (memory_available_jobs LESS 1)
+    set(memory_available_jobs 1)
+  endif()
+  if (memory_available_jobs LESS NUMBER_OF_LOGICAL_CORES)
+    set(LLVM_PARALLEL_LINK_JOBS "${memory_available_jobs}")
+  else()
+    set(LLVM_PARALLEL_LINK_JOBS "${NUMBER_OF_LOGICAL_CORES}")
+  endif()
+endif()
 if(CMAKE_GENERATOR MATCHES "Ninja")
   if(NOT LLVM_PARALLEL_LINK_JOBS AND uppercase_LLVM_ENABLE_LTO STREQUAL "THIN")
     message(STATUS "ThinLTO provides its own parallel linking - limiting parallel link jobs to 2.")

@jusito
Copy link

jusito commented Aug 15, 2022

For anyone interested, update the recipe but wasnt sure to create a pr to conan-center or to rdeterres work.

  • Fixed the issue with component.json not generated
  • merged the recipe with llvm-core
  • used ninja with job pool calculation / limitation
  • fixed openmp / libclang not generated or .so only in static builds and vice versa
  • added an regex option to keep matching binaries (very handy for some projects)

Just one note, for this part i have no idea how to fix it or if its needed:

self.cpp_info.components[component].build_modules["cmake_find_package"].extend([
    self._alias_module_file_rel_path,
    self._old_alias_module_file_rel_path,
])
self.cpp_info.components[component].build_modules["cmake_find_package_multi"].extend([
    self._alias_module_file_rel_path,
    self._old_alias_module_file_rel_path,
])

@tcanabrava
Copy link
Contributor

I Kindly request help from the conan team here.

LLVM is a really important piece of stack and we are trying to merge this request since at least LLVM 9. I took over an abandoned MR and ported to a newer LLVM version, then abandoned if after a while, someone took over, ported to another newer LLVM version, then abandoned it after a while, then someone else took over...

I'm currently using different bash / powershell scripts to download / compile llvm with the flags that I need on all systems bypassing conan, and that's really not something that I should be worried about doing.

Should we keep pushing this MR, should we wait for conan2, what should we do here?
@rdeterre , @Manu343726 , @jusito and all?

@ruilvo
Copy link
Contributor

ruilvo commented Oct 5, 2022

@tcanabrava

Fun you say that... I gave up on trying to package llvm/clang for Conan exactly because it doesn't support a workflow that I'd like... I'd like for it to be possible for Conan to download or build clang, and then use that just built compiler to all the other dependencies and the project itself... But that can't be done... So unless you're using the llvm libraries, packaging compilers isn't useful.

@tcanabrava
Copy link
Contributor

@tcanabrava

Fun you say that... I gave up on trying to package llvm/clang for Conan exactly because it doesn't support a workflow that I'd like... I'd like for it to be possible for Conan to download or build clang, and then use that just built compiler to all the other dependencies and the project itself... But that can't be done... So unless you're using the llvm libraries, packaging compilers isn't useful.

That's exactly what I'm doing, using libclang and libllvm. I'm not trying to use the binaries as compilers.

@jusito
Copy link

jusito commented Oct 5, 2022

I would be happy to see llvm in public conan. I had no time for a look into conan 2, so I have no idea how long it will take.
Which state of llvm recipe are you using right now?

We are using the version from my pr daily, updated it for 15.0.0. There you have the possibility to keep certain binaries clang / clang++ aso which we are using to decouple our application from a specific compiler. We are resolving the current llvm binary folder from conan and using clang to compile test files to llvm IR. So I can work with clang 15 but the project is using clang-14 for IR file generation. Secondly cmake can be used from conan with conan install cmake/3.23.3@ -g virtualrunenv, so using clang in the same way seems possible for me, I just have no idea how to support virtualrunenv generator. If you want to compile with clang, maybe including conan once for llvm/clang dep, extracting path, setting compiler, rerun autodetect + add other dependencies second?

Ok tested it with my state of the recipe: @ruilvo

  1. exported the recipe from my pr to llvm/14.0.6@phasar/develop
  2. conan install llvm/14.0.6@phasar/develop -s build_type=Release -o llvm:keep_binaries_regex="^(clang|clang\+\+|opt)$" -g virtualrunenv
  3. source activate_run.sh
  4. conan profile shouldn't set CC/CXX
  5. your environment CC/CXX should point to clang/clang++
  6. compile your application, clang 14.0.6 from conan is used instead of 15.0.0, even if conan profile is set to 15.

@bmanga
Copy link
Contributor

bmanga commented Nov 5, 2022

I too would like to see an llvm recipe in conan

@SSE4 SSE4 removed their assignment Nov 20, 2022
kevinhartman added a commit to openqasm/qe-compiler that referenced this pull request Feb 11, 2023
The compiler can now be built standalone much more easily using the
Conan package manager for dependencies.

### Dependencies and packaging
A `conanfile.py` and corresponding `conandata.yml` have been added at
the repo root. These files are used by the Conan package manager to
determine which external libraries are needed to build the compiler. In
the build steps outlined below, `conan install` is used to instruct
Conan to fetch and build necessary libraries. The `conanfile.py` also
creates a Conan package for the compiler itself, called `qss-compiler`,
which can be installed by other projects.

All external libraries exist on [Conan
center](https://conan.io/center/), except for:
- llvm-core
- clang-tools-extra
- qasm

For these libraries, we provide in-tree Conan recipes, which must be
registered with your host's Conan installation. To do this, run
`conan_deps.sh`. Run this any time you modify any of these in-tree
recipes.

There are two primary reasons we maintain these recipes in-tree:
1. They aren't available in Conan Center.
2. We update them more quickly than would be suitable for Conan Center
(e.g. it takes [months to
years](conan-io/conan-center-index#7613) to get
folks on Conan Center to agree on `llvm-core` version bumps).

### Building
Follow these steps to install dependencies with Conan and build the
compiler.

1. Run `conan_deps.sh` to export local Conan recipes for `llvm-core`,
`clang-tools-extra` and `qasm`.
3. Create a `build` directory and enter it.
4. Run `conan install --build=outdated <path to repo root>`
5. Run `conan build <path to repo root>` or use CMake to generate your
favorite build system and invoke that.
6. Run tests with `ninja check-tests` (build system is `ninja` by
default, use your own if relevant).

If you don't want to use Conan, make sure that you configure your system
with the necessary dependencies and tell CMake where to find
corresponding `find_package` modules (e.g. by specifying
`-DCMAKE_TOOLCHAIN_FILE=<path to toolchain>`).

### CI
CI is now available for Linux x64 to validate builds, unit tests, and
LLVM LIT testing for QASM and the mock target. This project builds both
LLVM Core and Clang Tools Extra (via Conan recipes), which takes several
hours. To accelerate development, a GitHub Cache Action is used to cache
the resulting library binaries along with all other Conan dependencies.
Due to GitHub's cache size limit, there's only enough space to cache
builds on the `main` branch. However, PR builds will load the Conan
cache from `main`, so they should finish in around 10 minutes. PRs that
change the LLVM recipes will need to wait for a full build (~5 hours).
Once a PR is merged to main, any changes to Conan files in the
repository will trigger a full build / cache rebuild to ensure the cache
is cleared of any stale dependencies (to prevent unbounded growth).
@jusito
Copy link

jusito commented Mar 8, 2023

Edit: continued / current state here: #17509 (comment)

@jusito jusito mentioned this pull request May 10, 2023
3 tasks
'LLVM_ENABLE_LIBPFM': False,
'LLVM_ENABLE_LIBEDIT': False,
'LLVM_ENABLE_FFI': self.options.with_ffi,
'LLVM_ENABLE_ZLIB': self.options.get_safe('with_zlib', False),
Copy link
Contributor

Choose a reason for hiding this comment

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

We should set these variables to FORCE_ON instead of True so they add the REQUIRED keyword. Otherwise if CMake fails to find the dependencies they get silently ignored.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the code form version 16.0.03

if(LLVM_ENABLE_ZLIB)
  if(LLVM_ENABLE_ZLIB STREQUAL FORCE_ON)
    find_package(ZLIB REQUIRED)
  elseif(NOT LLVM_USE_SANITIZER MATCHES "Memory.*")
    find_package(ZLIB)
  endif()
  if(ZLIB_FOUND)
  ...

Copy link

Choose a reason for hiding this comment

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

Its also there in in llvm 13 and documentation showed that this is the case for other options as well. Can add it in my pr.

@jcar87
Copy link
Contributor

jcar87 commented Apr 30, 2024

We are currently not prioritising packages that add toolchains (rather than libraries or pure executables), as we are prioritising other issues in our backlog.

LLVM itself has a myriad of components, and I suspect different users would have different needs. We are opening this issue to track this: #23834 - actual uses cases with examples will help the team gain better insights at how a potential llvm recipe may be used, whether or not it is worth packaging different LLVM components in separate recipes, and so on. Any information of this would be appreciated in that issue so that we can move forward, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Waiting on tools or services belonging to the infra
Projects
None yet
Development

Successfully merging this pull request may close these issues.