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

enhance LLVM easyblock for compilation of clang/flang + other llvm-projects #3373

Open
wants to merge 79 commits into
base: develop
Choose a base branch
from

Conversation

Crivella
Copy link
Contributor

@Crivella Crivella commented Jun 25, 2024

This EB is a modified version of the current Clang one (whether to keep them separate, having a simpler EB for Clang-only, or rename Clang to LLVMcore or viceversa is up for debate).

Requires testing of:

It enables building most of the main LLVM projects. What is still missing is/has not been tested:

  • libc
  • libclc
  • llvm-libgcc
  • pstl

The current EB improves on the Clang one by:

  • Implementing new version of flags for version of LLVM >= 18
  • Implement running the test-suite for all LLVM projects (with an EC specified allowed number of failures)
  • Implement both a bootstrapped and non-bootstrapped version of the toolchain
  • Implement a LLVM only build (bootstrapped only) where all dependencies from GCC are removed at the final stage

Currently/WIP:

  • The full_llvm build finishes successfully with 74 out of 119128 tests failing

    • 51 failures in bolt
    • 1 failure in clang tools
    • 18 failures in mlir
    • 3 failures in lldb
    • 1 failure in ompd
    • Same set of failures without the MLIR ones and only 1 lldb with a static build (55 out of 119090)
  • the bootstrapped non full_llvm build finishes successfully with 46 out of 119897 tests failing

    • 2 failures in bolt
    • 1 failure in clang tools
    • 18 failures in mlir
    • 21 failures in MemProfiler-x86_64-linux
    • 3 failures in lldb
    • 1 failure in ompd
  • the non bootstrapped version builds successfully but not build the test-suite (running make -j 1 check-all inside the builddir
    worked leading to 52 failures + 2 timeout out of 119900) (NOTE: the number of MemProfiler and timeout failure is not consistent)

    • Note: arguable benefit of the non-bootstrapped build as it took almost the same time as a 3 stage bootstrapped one
      The last stage with all the required projects is always the longest and it seems that clang is faster at building (at-least within the scope of the LLVM project)
    • Test if problem is with make -j XXX (!=1)
      • UPDATE: Added a step to try compiling the tests with -j 1 if the parallel build fails and the installation runs successfully
    • Investigate further failures once the build runs successfully with EB only
    • 2 failures in bolt
    • 1 failure in clang tools
    • 18 failures in mlir
    • 25 failures in MemProfiler-x86_64-linux (this number is not consistent across builds)
    • 5 failures in lldb
    • 1 failure in ompd
  • Improving sanity check depending on enabled options

Notes:

  • Building a simple C++ openmp program using iostream seems to work both with libc++ and libstdc++
    (tested with the full_llvm build and the bootstrapped non full one)
  • For both builds an hello world openmp program compiles with both libomp and libgomp but the one compiled with
    libgomp does not seem to recognize OMP_NUM_THREADS and runs with 1 thread.
    Tested with C (clang), C++ (clang++) and fortran (flang-new)

Possible TODOs:

  • Test GPU offloading by building and running on a system with GPU
    • For now works only with clang (Tested on VEGA front-end with an A100)
    • See this discourse post for flang-new
  • Test RPATH wrappers

easybuild/easyblocks/l/llvmcore.py Outdated Show resolved Hide resolved
easybuild/easyblocks/l/llvmcore.py Outdated Show resolved Hide resolved
easybuild/easyblocks/l/llvmcore.py Outdated Show resolved Hide resolved
@boegel boegel added the new label Jul 3, 2024
@boegel boegel added this to the release after 4.9.2 milestone Jul 3, 2024
@boegel
Copy link
Member

boegel commented Jul 3, 2024

I haven't looked at this in detail, but I'm not a big fan of "forking" an existing easyblock, and then updating it. There seems to be a lot of overlap still with the custom easyblock for Clang.

I do understand that the other option (updating the existing custom easyblock for Clang to introduce updates/enhancements) is painful too though, partially since there's a lot of history there.

@Crivella How difficult would it be to integrate the functionality you need in the existing easyblock for Clang (or LLVM, I'm not even sure which one is most relevant here, but I guess it's the Clang one).

@Crivella
Copy link
Contributor Author

Crivella commented Jul 3, 2024

@boegel I will try to work on it as soon as I finish all the current tests (there is still something to iron out on non shared builds, and understanding if the test failures should be addressed or can be ignored).
I went this route originally as I was working both on a standalone toolchain for building the single components separately and this one (and also for ease of testing features out). I will have to check how compatible the way the bootstrap was being done in the previous build with respect to this one, but it should be doable.

I do not know if it would still make sense to keep the Clang naming scheme as this would end up including also Flang and other optimization tools usable also with other frontends.

@ocaisa
Copy link
Member

ocaisa commented Jul 4, 2024

@boegel Isn't it a lot to ask to merge these easyblocks? The reality is that we already have two other forks for LLVM and AOMP, so the precedent already exists for taking a new/alternative path.

@Crivella
Copy link
Contributor Author

Crivella commented Jul 4, 2024

@boegel
I've been looking back at the 2 EB side by side and while not impossible to merge them, i think the most feasible approach would be the QE route of calling a different EB based on the version of the software.
While a lot of pieces have been taken from the Clang EB, here I've taken a fundamentally different approach for a few options and for the bootstrap build which would require deprecating options from the old EB if a new version is used.
Concerning the bootstrap build, in this EB, not all tools are being built at all stages of the build (only the minimally required one) and different options are being passed at different stages which would require rewriting half of the Clang EB to merge.

There is also the fact that for the Clang EB, flang is just an optional part of the build to be manually specified through the llvm_projects extra option.
The idea behind this EB is not to build just Clang but all the tooling required to have an LLVM toolchain in place which would require both clang and flang to always be present

@ocaisa
Copy link
Member

ocaisa commented Jul 4, 2024

One thing I don't actually like is the name, clang.py, because it is not just clang, it is LLVM with front end compilers. That is part of the complication, because in reality it should be llvm.py, and clang.py should stop being used. QE-style forking within the relevant easyblocks seems like the only available option.

@Crivella
Copy link
Contributor Author

Crivella commented Jul 4, 2024

As an update, i've also tested now with all tools enabled and build_targets = ['all'].
The compilation + test_suite was successful with only a slight increase in errors (with respect to the previous bootstrap+full_llvm build with default build target):

  • 77 errors out of 119238 tests
    • 50 due to bolt
    • 6 due to clang + PowerPC architecture
    • 1 due to clang-tools
    • 18 due to mlir
    • 1 due to lldb
    • 1 due to ompd

It should be noted that before 34341 (28.84%) were flagged as unsupported while now only 4588 (3.85%) which should be due to the experimental archs

@Crivella
Copy link
Contributor Author

Crivella commented Jul 8, 2024

UPDATE

Testing with openmp offloading on VEGA frontend node:

  • AMD EPYC 7452 (Zen2 Rome)
  • NVIDIA A100

With:

build_targets = ['X86', 'NVPTX']
cuda_compute_capabilities = ['8.0']

I encountered extra errors related to libomptarget:

  • 270 due to x86 (out of 275)
  • 11 due to nvptx (out of 302)

As a sanitycheck I also recompiled on my WS by allowing libomptarget to be compiled for x86 (before i had it enabled only for nvptx and amd) and i get a similar number of test errors (268)

Beside that i tried to compile a simple openmp application and the GPU offloading seems to be working correctly (verified the GPU is being used from nvtop)

1 similar comment
@Crivella
Copy link
Contributor Author

Crivella commented Jul 8, 2024

UPDATE

Testing with openmp offloading on VEGA frontend node:

  • AMD EPYC 7452 (Zen2 Rome)
  • NVIDIA A100

With:

build_targets = ['X86', 'NVPTX']
cuda_compute_capabilities = ['8.0']

I encountered extra errors related to libomptarget:

  • 270 due to x86 (out of 275)
  • 11 due to nvptx (out of 302)

As a sanitycheck I also recompiled on my WS by allowing libomptarget to be compiled for x86 (before i had it enabled only for nvptx and amd) and i get a similar number of test errors (268)

Beside that i tried to compile a simple openmp application and the GPU offloading seems to be working correctly (verified the GPU is being used from nvtop)

@Crivella Crivella force-pushed the feature-LLVM_unified branch 2 times, most recently from 8c906e5 to 3b348e2 Compare July 9, 2024 15:48
@Thyre
Copy link
Contributor

Thyre commented Jul 10, 2024

I've tried to build LLVM using your easyblock and easyconfig PRs on a fresh devel installation. I had some issues with Python, where the tests reported that Python itself was not found even though it was installed. After disabling the Python parts I had no trouble building both LLVMcore variants.

I haven't looked to deeply into which and how many tests failed though. I built everything on my local workstation (Intel Core i7-12700, AMD Radeon RX7700XT).

@Crivella
Copy link
Contributor Author

@Thyre

Thanks for the report, will look into it
The error was related to the python_bindings = True setting in the EC files?

@Thyre
Copy link
Contributor

Thyre commented Jul 10, 2024

@Thyre

Thanks for the report, will look into it The error was related to the python_bindings = True setting in the EC files?

Yes, exactly. When setting it to False, everything worked fine.
The steps to reproduce were basically:

  1. Clone the EB repos (with your PRs) (instructions taken from here)
  2. Run the following command: eb --robot LLVMcore-18.1.7.eb LLVMcore-18.1.7-GCCcore-13.3.0.eb --parallel 8

@Crivella
Copy link
Contributor Author

I had some issues with Python, where the tests reported that Python itself was not found even though it was installed.

I have an idea on what might be happening. Was the error during the test_step or the sanity_check_step.

In case it is the latter it is very likely that the reason would be that I had another python available and capable of working on my WS during the build which was causing the sanity check to pass even if python was not in the normal dependencies instead of the build ones.

Let me know if this is the case, in the meanwhile i will fix the EC files

@Crivella
Copy link
Contributor Author

Crivella commented Jul 18, 2024

UPDATE:

after some discussion in the 2024/07/17 Easybuild Conference Call, it was decided to replace the EB_LLVM easyblock with the new one from this PR.

To this end, the following has been added:

  • a minimal build option has been added to compile LLVM only.
  • A bypass of the version checks LLVM>=18.1.6 and GCCcore>=13.3.0 when minimal is enabled to allow building of older versions
    (all the CMake variable are the same and a few extra ones will just be ignored)
    Tested manually up to version 14.0.3
  • Modified PR on EC to add new version of LLVM and modify the old ones to use the new easyblock starting from version 11.1.0
    (The older ones make use ofr archived/depcrecated toolchains) (https://docs.easybuild.io/policies/toolchains/)

The following tests have been manually performed for sanity check:

  • Build of the following LLVM versions:
    • 14.0.3
    • 14.0.6-lite
    • 16.0.6
  • Build of packages actively depending on LLVM:
    • LDC-1.30.0-GCCcore-11.3.0.eb <-- ('LLVM', '14.0.3')
    • LDC-1.36.0-GCCcore-12.3.0.eb <-- ('LLVM', '16.0.6')
    • umap-learn-0.5.3-foss-2022a.eb: <-- ('LLVM', '14.0.3'),
    • umap-learn-0.5.5-foss-2023a.eb <-- ('LLVM', '16.0.6')
    • numba-0.58.1-foss-2023a.eb <-- ('LLVM', '14.0.6', '-llvmlite')
    • Mesa-22.0.3-GCCcore-11.3.0.eb <-- ('LLVM', '14.0.3')
    • Mesa-23.1.4-GCCcore-12.3.0.eb <-- ('LLVM', '16.0.6')

@ocaisa ocaisa changed the title LLVMcore easyblock for compilation of clang/flang + other llvm-projects LLVM easyblock update for compilation of clang/flang + other llvm-projects Jul 18, 2024
@Crivella Crivella force-pushed the feature-LLVM_unified branch 2 times, most recently from 2349af1 to 21ad3f5 Compare July 25, 2024 08:23
@boegel boegel changed the title LLVM easyblock update for compilation of clang/flang + other llvm-projects enhance LLVM easyblock for compilation of clang/flang + other llvm-projects Jul 31, 2024
@boegel boegel added enhancement and removed new labels Jul 31, 2024
Comment on lines 97 to 333
unknown_targets = [target for target in build_targets if target not in CLANG_TARGETS]
unknown_targets = set(build_targets) - set(ALL_TARGETS)

if unknown_targets:
raise EasyBuildError("Some of the chosen build targets (%s) are not in %s.",
', '.join(unknown_targets), ', '.join(CLANG_TARGETS))
', '.join(unknown_targets), ', '.join(ALL_TARGETS))
exp_targets = set(build_targets) & set(LLVM_EXPERIMENTAL_TARGETS)
if exp_targets:
self.log.warning("Experimental targets %s are being used.", ', '.join(exp_targets))

self.cfg.update('configopts', '-DLLVM_TARGETS_TO_BUILD="%s"' % ';'.join(build_targets))
self.build_targets = build_targets or []

if LooseVersion(self.version) >= LooseVersion('15.0'):
# make sure that CMake modules are available in build directory,
# by moving the extracted folder to the expected location
cmake_modules_path = os.path.join(self.builddir, 'cmake-%s.src' % self.version)
if os.path.exists(cmake_modules_path):
move_file(cmake_modules_path, os.path.join(self.builddir, 'cmake'))
self.nvptx_cond = 'NVPTX' in self.build_targets
self.amd_cond = 'AMDGPU' in self.build_targets
self.all_cond = 'all' in self.build_targets
self.cuda_cc = []
if self.nvptx_cond or self.all_cond:
# list of CUDA compute capabilities to use can be specifed in two ways (where (2) overrules (1)):
# (1) in the easyconfig file, via the custom cuda_compute_capabilities;
# (2) in the EasyBuild configuration, via --cuda-compute-capabilities configuration option;
ec_cuda_cc = self.cfg['cuda_compute_capabilities']
cfg_cuda_cc = build_option('cuda_compute_capabilities')
cuda_cc = cfg_cuda_cc or ec_cuda_cc or []
if not cuda_cc and self.nvptx_cond:
raise EasyBuildError(
"Can't build Clang with CUDA support without specifying 'cuda-compute-capabilities'"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please correct me if I'm wrong:

The current approach would just take the host architecture and not NVPTX / AMDGPU if the user doesn't explicitly set anything in their EasyConfig, right? Would it make sense to use the approach from the Clang EasyBlock or detect enabling them in another way (e.g. via cuda_compute_capabilities)?

if build_targets is None:
   deps = [dep['name'].lower() for dep in self.cfg.dependencies()]
   arch = get_cpu_architecture()
   try:
       default_targets = DEFAULT_TARGETS_MAP[arch][:]
       # If CUDA is included as a dep, add NVPTX as a target
       # There are (old) toolchains with CUDA as part of the toolchain
       cuda_toolchain = hasattr(self.toolchain, 'COMPILER_CUDA_FAMILY')
       if 'cuda' in deps or cuda_toolchain:
           default_targets += ['NVPTX']
       # For AMDGPU support we need ROCR-Runtime and
       # ROCT-Thunk-Interface, however, since ROCT is a dependency of
       # ROCR we only check for the ROCR-Runtime here
       # https://openmp.llvm.org/SupportAndFAQ.html#q-how-to-build-an-openmp-amdgpu-offload-capable-compiler
       if 'rocr-runtime' in deps:
           default_targets += ['AMDGPU']
       self.cfg['build_targets'] = build_targets = default_targets
       self.log.debug("Using %s as default build targets for CPU/GPU architecture %s.", default_targets, arch)
   except KeyError:
       raise EasyBuildError("No default build targets defined for CPU architecture %s.", arch)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes currently only the host architecture is used by default.

What the Clang EB does is just check if cuda or rocr-runtime is in the dependencies.
I think atleast for CUDA i had removed it as i was able to get offload going without adding CUDA to the deps (will have to double check as i did it a couple months ago and don't have access to the frontend where i tested this).

For using cuda_compute_capabilities i followed the same approach as the Clang EB of checking that it is defined when NVPTX is enabled as a target and set the proper variables (or fail if it is not defined).

I guess it would make sense to automatically enable NVPTX/AMDGPU if the respective deps are specified in the EC or if cuda_compute_capabilities/amd_gfx_list are defined

I am thinking the advantage of the automatic detection would be to be able to have NVPTX/AMDGPU enabled without having to also manually specify the CPU target.
I can try adding the original check and see how it goes

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it would make sense to automatically enable NVPTX/AMDGPU if the respective deps are specified in the EC or if cuda_compute_capabilities/amd_gfx_list are defined

For our systems at JSC, I added this small snippet to our "custom" LLVM EasyBlock and used your second idea to check if NVPTX should be built:

cuda_cc_list = build_option('cuda_compute_capabilities') or self.cfg['cuda_compute_capabilities']
if cuda_cc_list:
    build_targets += ["NVPTX"]
    self.log.debug("Add NVPTX as additional build target architecture")

The code is not perfect, but is likely sufficient for our use case. LLVM does not require having CUDA / ROCm to build the respective NVPTX / AMDGPU backends as far as I am aware of, only when actually using them.

I am thinking the advantage of the automatic detection would be to be able to have NVPTX/AMDGPU enabled without having to also manually specify the CPU target.

Agreed, this was my thought as well. Especially if one has a "global" repository for multiple systems with different architectures, needing to specify the CPU target each time might get messy. Users are also still able to specify the targets manually, as the automatic detection is only done when build_targets is not defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've implemented the automatic detection based on having cuda_compute_capabilities or amd_gfx_list defined (cac8fe9).

For now this is being done only if no build_targets are being manually specified.
It is also true that if someone is specifying either of the 2 variables they might want to have the target enabled, but i feel giving priority should still be given to build_targets (Maybe could add a check for printing a warning or outright raising an error if cuda_cc/amd_gfx are defined and the corresponding targets are not).

Copy link
Contributor

Choose a reason for hiding this comment

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

I think your current approach is a good idea, only enabling the backends if the user has not defined which ones to build. Adding a warning (or at least a log entry) if he might've missed one might be a good idea.

@boegel boegel modified the milestones: 4.9.3, release after 4.9.3 Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants