Skip to content

use property for gcc_prefix/gcc_root in LLVM easyblock #3793

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

Merged
merged 2 commits into from
Jul 10, 2025

Conversation

Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Jun 23, 2025

As discussed this uses properties that auto-initialize on (first) use to avoid them being used before setting in _set_gcc_prefix which acted as a configure-method AND setting those properties

BUGFIX (Separated out to #3817 but would cause a conflict)
For bootstrap = False the function _create_compiler_config_file was called without checking the version which results in adding a CLI argument for clang compilers that was introduced in LLVM 19 which causes failures.

@Flamefire Flamefire force-pushed the llvm-property branch 2 times, most recently from d57c49b to dc7c566 Compare June 24, 2025 09:28
@Flamefire Flamefire changed the title Use property for gcc_prefix/gcc_root in LLVM easyblock Fix bootstrap for LLVM < 19 and use property for gcc_prefix/gcc_root in LLVM easyblock Jul 1, 2025
@Flamefire Flamefire changed the title Fix bootstrap for LLVM < 19 and use property for gcc_prefix/gcc_root in LLVM easyblock Fix non-bootstrap build for LLVM < 19 and use property for gcc_prefix/gcc_root in LLVM easyblock Jul 1, 2025
@boegel boegel added the bug fix label Jul 1, 2025
@boegel boegel added this to the 5.1.1 milestone Jul 1, 2025
@Flamefire
Copy link
Contributor Author

Test report by @Flamefire

Overview of tested easyconfigs (in order)

Build succeeded for 2 out of 3 (3 easyconfigs in total)
n1623 - Linux RHEL 8.9 (Ootpa), x86_64, Intel(R) Xeon(R) Platinum 8470 (sapphirerapids), Python 3.9.18
See https://gist.github.com/Flamefire/89e8448451c51ac24bfe2d7bd27b90cc for a full test report.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire

Overview of tested easyconfigs (in order)

Build succeeded for 2 out of 3 (3 easyconfigs in total)
c49 - Linux AlmaLinux 9.4, x86_64, AMD EPYC 9334 32-Core Processor (zen4), 4 x NVIDIA NVIDIA H100, 560.35.03, Python 3.9.18
See https://gist.github.com/Flamefire/4ffc79eb45a73df765fa1d69e72ee110 for a full test report.

@boegel
Copy link
Member

boegel commented Jul 2, 2025

@Flamefire Merge conflict to fix now that #3817 is merged?

@boegel boegel changed the title Fix non-bootstrap build for LLVM < 19 and use property for gcc_prefix/gcc_root in LLVM easyblock use property for gcc_prefix/gcc_root in LLVM easyblock Jul 2, 2025
@boegel boegel modified the milestones: 5.1.1, release after 5.1.1 Jul 3, 2025
@Flamefire
Copy link
Contributor Author

Rebased

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire

Overview of tested easyconfigs (in order)

Build succeeded for 2 out of 3 (3 easyconfigs in total)
n1540 - Linux RHEL 8.9 (Ootpa), x86_64, Intel(R) Xeon(R) Platinum 8470 (sapphirerapids), Python 3.9.18
See https://gist.github.com/Flamefire/0c772315a292d0fd3839bae8a9c4dd44 for a full test report.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire

Overview of tested easyconfigs (in order)

  • SUCCESS LLVM-19.1.7-GCCcore-13.3.0.eb
  • SUCCESS LLVM-20.1.5-GCCcore-13.3.0.eb
  • SUCCESS LLVM-18.1.8-GCCcore-13.3.0.eb

Build succeeded for 3 out of 3 (3 easyconfigs in total)
c83 - Linux AlmaLinux 9.4, x86_64, AMD EPYC 9334 32-Core Processor (zen4), 4 x NVIDIA NVIDIA H100, 560.35.03, Python 3.9.18
See https://gist.github.com/Flamefire/a602c8f8c30061b8362f4bed6c7a001c for a full test report.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire

Overview of tested easyconfigs (in order)

  • SUCCESS LLVM-19.1.7-GCCcore-13.3.0.eb
  • SUCCESS LLVM-20.1.5-GCCcore-13.3.0.eb
  • SUCCESS LLVM-18.1.8-GCCcore-13.3.0.eb

Build succeeded for 3 out of 3 (3 easyconfigs in total)
i7005 - Linux Rocky Linux 8.9 (Green Obsidian), x86_64, AMD EPYC 7702 64-Core Processor (zen2), Python 3.9.18
See https://gist.github.com/Flamefire/eeccc718e59e802a66519b9a2911c76f for a full test report.


def _set_gcc_prefix_probs(self):
"""Set properties of currently loaded GCC installation"""
self._gcc_root, self._gcc_prefix = self._get_gcc_prefix()
Copy link
Contributor

Choose a reason for hiding this comment

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

I would put this inside an if as it was done originally.
I do not foresee any situation where these variables would change during the build process

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a method only used on access of the properties to set the "backing storage". So the if is outside this function

I agree they won't change, so there is no harm if this method is (wrongly) called again

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no if either inside gcc_prefix nor gcc_root so the method self._get_gcc_prefix() will be called every time one of these 2 properties are requested

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, thanks! Seems I couldn't decide where to put the if and forgot it in the end.
Added

@Crivella
Copy link
Contributor

Crivella commented Jul 9, 2025

@boegelbot please test @ jsc-zen3
EB_ARGS="LLVM-16.0.6-GCCcore-12.3.0.eb LLVM-18.1.8-GCCcore-13.3.0.eb LLVM-19.1.7-GCCcore-13.3.0.eb LLVM-20.1.5-GCCcore-13.3.0.eb"
CORE_CNT=16

@boegelbot
Copy link

@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ develop != 'develop' ]]; then EB_BRANCH=develop ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/develop source init_env_easybuild_develop.sh; fi; EB_PR=3793 EB_ARGS="LLVM-16.0.6-GCCcore-12.3.0.eb LLVM-18.1.8-GCCcore-13.3.0.eb LLVM-19.1.7-GCCcore-13.3.0.eb LLVM-20.1.5-GCCcore-13.3.0.eb" EB_REPO=easybuild-easyblocks EB_BRANCH=develop /opt/software/slurm/bin/sbatch --job-name test_PR_3793 --ntasks="16" ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 7176

Test results coming soon (I hope)...

- notification for comment with ID 3051630099 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegelbot
Copy link

Test report by @boegelbot

Overview of tested easyconfigs (in order)

  • SUCCESS LLVM-16.0.6-GCCcore-12.3.0.eb
  • SUCCESS LLVM-18.1.8-GCCcore-13.3.0.eb
  • SUCCESS LLVM-19.1.7-GCCcore-13.3.0.eb
  • SUCCESS LLVM-20.1.5-GCCcore-13.3.0.eb

Build succeeded for 4 out of 4 (4 easyconfigs in total)
jsczen3c1.int.jsc-zen3.fz-juelich.de - Linux Rocky Linux 9.5, x86_64, AMD EPYC-Milan Processor (zen3), Python 3.9.21
See https://gist.github.com/boegelbot/086c4ccd059dd43134da06852e278610 for a full test report.

Copy link
Contributor

@Crivella Crivella left a comment

Choose a reason for hiding this comment

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

LGTM

@Crivella
Copy link
Contributor

Going in, thanks @Flamefire!

@Crivella Crivella merged commit 1cfb4d7 into easybuilders:develop Jul 10, 2025
17 checks passed
@Flamefire Flamefire deleted the llvm-property branch July 10, 2025 11:03
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.

4 participants