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

[BOLT] Add builder #8391

Merged
merged 24 commits into from
Apr 17, 2024
Merged

[BOLT] Add builder #8391

merged 24 commits into from
Apr 17, 2024

Conversation

Zentrik
Copy link
Contributor

@Zentrik Zentrik commented Mar 30, 2024

No description provided.

 Tested locally with x86_64-linux-gnu-cxx11.
In the discord, I see someone built on FreeBSD and seems to build on windows here: llvm/llvm-project#73085.
I haven't tested locally, as this takes hours to build for me.
Might need a patch for windows: llvm/llvm-project#73006.
Not sure why the code in common.jl uses `-gt` so much.
@Zentrik
Copy link
Contributor Author

Zentrik commented Mar 30, 2024

Should LLVM_full@14.0.5, LLVM_full_assert@14.0.5 be deleted?

Seems like you cannot build on it and it shouldn't even exist in Yggdrasil.
@Zentrik

This comment was marked as outdated.

@Zentrik

This comment was marked as outdated.

@Zentrik
Copy link
Contributor Author

Zentrik commented Mar 31, 2024

I'll probably just restrict to LLVM >= 16 given all the failures.

@giordano
Copy link
Member

It's quite unlikely we're going to rebuild/use anything older than v16.

@Zentrik Zentrik changed the title Add BOLT to LLVM_full for versions >= 14 Add BOLT to LLVM_full for versions >= 16 Apr 1, 2024
FreeBSD fails even with this patch and we've disabled building runtime as it's not supported.
So this patch is useless.
This reverts commit 9aa1a07.
@Zentrik Zentrik marked this pull request as ready for review April 1, 2024 09:58
@Zentrik Zentrik marked this pull request as draft April 1, 2024 11:54
@Zentrik
Copy link
Contributor Author

Zentrik commented Apr 1, 2024

Feel free to cancel the build if you need to free up some runners.

Not sure if there's a better approach. I don't know if calling build_tarballs twice will cause problems.
@Zentrik Zentrik marked this pull request as ready for review April 14, 2024 11:51
@Zentrik
Copy link
Contributor Author

Zentrik commented Apr 14, 2024

I'm pretty sure a6e33e1 should have fixed the error about not being able to find llvm-bolt so I'm a bit confused. Is this some weird caching behaviour?

@vchuravy
Copy link
Member

I am very hesitant with taking on extra tools into the core LLVM build. We have enough issues with the current set and getting to LLVM 17

Could the build of BOLT be separated from the core LLVM compiler?

@giordano
Copy link
Member

@Zentrik if you need inspiration for a standalone llvm package you can have a look at https://github.com/JuliaPackaging/Yggdrasil/blob/24010c93241559f44b967496e65782a9d18729f0/L/LLVMOpenMP/build_tarballs.jl, although that's really not different from any other standard package. Only caveat is that my understanding is that nowadays llvm component should always be built out of the (giant) git monorepo, rather than using the individual release tarballs (which at least can be downloaded in a finite time), so

"https://github.com/llvm/llvm-project/releases/download/llvmorg-$(version)/openmp-$(version).src.tar.xz",
is probably not good style.

@vchuravy
Copy link
Member

Yeah if this is not feasible and we have a strong use-case for why then I would also be happy to add it, but we should't be too hastly.

Another example would be JuliaLang/julia#52945 or the old RV/Polly integration. First prepare a PR that shows the value on Julia base and then we can add it here, but if this is for a personal experiment the maintenance burden needs be considered.

@Zentrik Zentrik changed the title Add BOLT to LLVM_full for versions >= 16 [BOLT] Add builder Apr 16, 2024
Cross compilation for it seems to be broken, llvm/llvm-project#84547.
Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

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

The resulting tarballs is very large, it's over 400 MB for x86_64 linux and it contains loads of static archives which presumably are unnecessary, and the bin/ directory is full of executables, I thought you only needed llvm-bolt?

B/BOLT/build_tarballs.jl Outdated Show resolved Hide resolved
B/BOLT/build_tarballs.jl Outdated Show resolved Hide resolved
B/BOLT/build_tarballs.jl Outdated Show resolved Hide resolved
B/BOLT/build_tarballs.jl Outdated Show resolved Hide resolved
Cuts tarball size from ~400 mb to 32mb. Untarred llvm-bolt and llvm-bolt-heatmap are both 42mb and 99% of size.
B/BOLT/build_tarballs.jl Outdated Show resolved Hide resolved
Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

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

This is looking good to me now, only a couple of more minor comments. Anyone else wants to have a look?

B/BOLT/build_tarballs.jl Outdated Show resolved Hide resolved
B/BOLT/build_tarballs.jl Outdated Show resolved Hide resolved
B/BOLT/build_tarballs.jl Outdated Show resolved Hide resolved
Might as well, won't be hard to adjust if they are removed at some point.
@giordano giordano merged commit 6357f54 into JuliaPackaging:master Apr 17, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants