Skip to content

llvm: add missing binutils and gcc-runtime dependency#3880

Open
rbberger wants to merge 4 commits intospack:developfrom
rbberger:fix/llvm-binutils-dep
Open

llvm: add missing binutils and gcc-runtime dependency#3880
rbberger wants to merge 4 commits intospack:developfrom
rbberger:fix/llvm-binutils-dep

Conversation

@rbberger
Copy link
Copy Markdown
Member

@rbberger rbberger commented Mar 18, 2026

Previously +gold pulled in binutils as dependency. That default was recently changed since gold is deprecated. See #3454.

However, building a custom GCC will typically pull in a newer binutils than what is on the system. Using such a GCC for building LLVM then fails if the system binutils is too old. We therefore need to make sure that LLVM also uses the newer binutils.

Also adds an explicit -B<binutils-prefix>/bin to the LLVM configuration files to it prefers using those binaries over defaulting to a location such as /usr/bin. Just having the binutils prefix in PATH is not good enough.

It further injects a gcc-runtime dependency for packages building with a llvm that is built with gcc.

@spackbot-triage spackbot-triage bot requested review from haampie, skosukhin and trws March 18, 2026 05:08
@rbberger rbberger force-pushed the fix/llvm-binutils-dep branch 2 times, most recently from da05909 to 124ac4b Compare March 18, 2026 07:31
@alalazo
Copy link
Copy Markdown
Member

alalazo commented Mar 18, 2026

Seems similar to what I had to do in #3600 for the OneAPI toolchain.

@rbberger
Copy link
Copy Markdown
Member Author

@alalazo also seems related to #3817. are we missing a llvm-runtime?

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Mar 18, 2026

Yeah, we don't have llvm-runtime atm

Previously +gold pulled in binutils as dependency. That default was recently
changed since gold is deprecated. See spack#3454.

However, building a custom GCC will typically pull in a newer binutils than
what is on the system. Using such a GCC for building LLVM then fails if the
system binutils is too old. We therefore need to make sure that LLVM also uses
the newer binutils.
Putting binutils in PATH isn't good enough due to LLVM lookup logic. Be
explicit by adding -B<binutils-prefix>/bin to prefer this prefix for ld.
@rbberger
Copy link
Copy Markdown
Member Author

rbberger commented Mar 18, 2026

@alalazo @haampie please take a closer look. I've needed to add -B to the LLVM build so it makes sure to prefer the newer binutils prefix over defaulting to /usr/bin/ld. Apparently, even if binutils is a dependency, that isn't good enough since the LLVM lookup logic considers PATH too late. I'm using -B over --ld-path, since the latter would prevent -fuse-ld=lld.

@pearzt
Copy link
Copy Markdown
Contributor

pearzt commented Mar 19, 2026

I just tested this with a fresh Spack installation and can confirm that it fixes the problem I reported in #3817 \o/

@haampie
Copy link
Copy Markdown
Member

haampie commented Mar 19, 2026

Looks good to me, I'm gonna leave this to @alalazo to look at since he's doing very similar work right now.

@rbberger rbberger changed the title llvm: add missing binutils dependency llvm: add missing binutils and gcc-runtime dependency Mar 20, 2026
@rbberger
Copy link
Copy Markdown
Member Author

@spackbot fix style

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Mar 20, 2026

Let me see if I can fix that for you!

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Mar 20, 2026

I was able to run spack style --fix for you!

spack style --fix
repos/spack_repo/builtin/build_systems/go.py
repos/spack_repo/builtin/build_systems/python.py
repos/spack_repo/builtin/packages/blis/package.py
repos/spack_repo/builtin/packages/cdo/package.py
repos/spack_repo/builtin/packages/dla_future/package.py
repos/spack_repo/builtin/packages/geant4/package.py
repos/spack_repo/builtin/packages/go/package.py
repos/spack_repo/builtin/packages/go_bootstrap/package.py
repos/spack_repo/builtin/packages/hdf5/package.py
repos/spack_repo/builtin/packages/llvm/package.py
repos/spack_repo/builtin/packages/llvm_amdgpu/package.py
repos/spack_repo/builtin/packages/octave_io/package.py
repos/spack_repo/builtin/packages/pika/package.py
repos/spack_repo/builtin/packages/py_tensorboard_data_server/package.py
repos/spack_repo/builtin/packages/rust/package.py
�[1;34m==> �[0mrunning flake8
repos/spack_repo/builtin/packages/llvm/package.py:357: [E501] line too long (104 > 99 characters)
�[1;34m==> �[0mrunning isort
�[1;34m==> �[0mrunning black
Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with style fixes.

prckent added a commit to prckent/qmcpack that referenced this pull request Apr 9, 2026
## Proposed changes

The installation and nightly scripts have been significantly updated for the modern era of spack where compilers mostly work as dependencies, environments are more functional, and the package repository is separate.

Note that the compilers must be installed (concretized) ahead of use, so we install those first, relying on the system compiler being good enough.

Several bugs are worked around, e.g. compilers are generally built with the system gcc. spack currently fails to inject newer binutils dependencies consistently so installing llvm using a spack built gcc is prone to failure. [ Solution is in progress spack/spack-packages#3880 ]

I was not able to make a working llvm v22.1.1 for offload. v21 is used for now (NV, host CPU).

Several packages need updating before we can make more use of them e.g. to avoid build failures with gcc@15

## What type(s) of changes does this code introduce?

- Testing changes (e.g. new unit/integration/performance tests)

### Does this introduce a breaking change?

- No

## What systems has this change been tested on?

nitrogen, nitrogen2, sulfur

## Checklist

* * [X] I have read the pull request guidance and develop docs
* * [X] This PR is up to date with the current state of 'develop'
* * [ ] Code added or changed in the PR has been clang-formatted
* * [ ] This PR adds tests to cover any new code, or to catch a bug that is being fixed
* * [ ] Documentation has been added (if appropriate)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants