Skip to content

Conversation

@Thyre
Copy link
Collaborator

@Thyre Thyre commented Sep 19, 2025

(created using eb --new-pr)

Required for ROCm-LLVM 7.0.0 to build, as its using a pre-release LLVM 20, but already includes the required changes.

@Thyre
Copy link
Collaborator Author

Thyre commented Oct 13, 2025

@Crivella, @Flamefire any objections to this?
This is basically to allow ROCm 7.0.0 to build. The build they're using already includes the fixes we need, but still identifies as LLVM 20.0. We could try to create a hacky workaround, or, if e.g. some other LLVM 20 fork doesn't include this fix, simply port the patch from LLVM upstream.

This certainly would make things easier than trying to break things again, only so that our EasyBlock applies 😅

@Crivella
Copy link
Contributor

I do not have a strong opinion on this.
For me this section was also not really needed to begin with since we already had parallel testing in LIT (see discussion on #3875).

Right now i really do not know how the builder behaves (EG if lit is aware there are multiple instances running and limit itselfs or not)

I've never really encountered problems but i've also not done many rebuilds of LLVM since and when i do i always use a limited number of CPUs to avoid memory problems which might be what is saving it.

@Thyre
Copy link
Collaborator Author

Thyre commented Oct 13, 2025

I haven't encountered any issues at least, though I have noticed that my system is more utilized than it was before that particular change and I rebuilt LLVM more often than I want to admit.

However, I never checked the actual CPU usage...

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

the official LLVM does not even have a 20.0 (20.x starts from x=1)
So this would only affect forks of LLVM that version LLVM as 20.0 or 20.0.x

Probably better to work around a failure in applying the patch to the cmake files if it is ever needed

@Crivella
Copy link
Contributor

Going in, thanks @Thyre!

@Crivella Crivella merged commit 917cea3 into easybuilders:develop Oct 14, 2025
17 checks passed
@Thyre Thyre mentioned this pull request Oct 24, 2025
4 tasks
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