Skip to content

[CMake] Simplify building of bolt-optimized clang for distribution builds #117772

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[CMake] Simplify building of bolt-optimized clang for distribution builds #117772

wants to merge 2 commits into from

Conversation

@tstellar
Copy link
Collaborator

@tstellar tstellar commented Nov 26, 2024

Currently if you want to do a distribution build (i.e. using LLVM_DISTIRIBUTION_COMPONENTS) with a bolt-optimized clang, then you need to pass two separate targets to ninja like this:

ninja -C build clang-bolt distribiution

This patch simplifies this workflow so that you can do a distribution build with a bolt-optimized build with just a single target:

ninja -C build distribution

@tstellar
[CMake] Simplify building of bolt-optimized clang for distribution bu…
99e42cf 
…ilds

Currently if you want to do a distribution build (i.e. using
LLVM_DISTIRIBUTION_COMPONENTS) with a bolt-optimized clang, then
you need to pass two separate targets to ninja like this:

ninja -C build clang-bolt distribiution

This patch simplifies this workflow so that you can do a
distribution build with a bolt-optimized build with just a single target:

ninja -C build distribution
@llvmbot llvmbot added the cmake Build system in general and CMake in particular label Nov 26, 2024
@petrhosek
Copy link
Member

petrhosek commented Nov 26, 2024

Could we also gate this on CLANG_BOLT?

@tstellar
Copy link
Collaborator Author

tstellar commented Nov 27, 2024

Could we also gate this on CLANG_BOLT?

Is that redundant with the check for the clang-bolt target?

@tstellar
Copy link
Collaborator Author

tstellar commented Nov 27, 2024

I noticed this wasn't working for the install targets, so I had to add clang-bolt as a dependency to the install targets too.

I wonder if would be better to have the install targets depend on the distribution target instead.

@tstellar
Copy link
Collaborator Author

tstellar commented Nov 27, 2024

As I look into this more, it's not strictly a problem with distribution builds. Just enabling bolt and running ninja install-clang does not give you a bolt optimized clang. I'm going to try to do a more generic patch.

@aaupov
Copy link
Contributor

aaupov commented Nov 27, 2024

As I look into this more, it's not strictly a problem with distribution builds. Just enabling bolt and running ninja install-clang does not give you a bolt optimized clang.

Right, for that matter, it's not triggered by ninja clang as well. There's a problem that clang-bolt depends on clang target, so clang-bolt can't be a provider for it. In theory we could decouple targets: if CLANG_BOLT is provided, pre-BOLT clang target could be called differently, and clang-bolt target could be the provider for clang. But I thought it would be too ambitious for me to change how clang is built, which is why clang-bolt is "on the side".

@tstellar
Copy link
Collaborator Author

tstellar commented Nov 27, 2024

Right, for that matter, it's not triggered by ninja clang as well. There's a problem that clang-bolt depends on clang target, so clang-bolt can't be a provider for it. In theory we could decouple targets: if CLANG_BOLT is provided, pre-BOLT clang target could be called differently, and clang-bolt target could be the provider for clang. But I thought it would be too ambitious for me to change how clang is built, which is why clang-bolt is "on the side".

OK, I was thinking of doing something like add_dependencies(install-clang clang-bolt) but maybe it would make more sense for me to rewrite the clang target to do the bolt optimization.

@tstellar
Copy link
Collaborator Author

tstellar commented Dec 13, 2024

Closing this in favor of #119896.

@tstellar tstellar closed this Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aaupov aaupov aaupov approved these changes

@smeenai smeenai smeenai approved these changes

@petrhosek petrhosek Awaiting requested review from petrhosek

@llvm-beanz llvm-beanz Awaiting requested review from llvm-beanz

Assignees

No one assigned

Labels

cmake Build system in general and CMake in particular

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tstellar @petrhosek @aaupov @smeenai @llvmbot