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

llvm: Re-enable PGO and update build configs #4570

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Conversation

ReillyBrogan
Copy link
Contributor

@ReillyBrogan ReillyBrogan commented Dec 12, 2024

Changes:

  • Re-enable building LLVM with PGO
  • Enable perf support in the JIT engine
  • Build MLIR-C library shared
  • Install the lld man page
  • Add "Solus $version-$release" to the Clang version
  • Disable building libcxx and llvm benchmarks (these weren't installed but were still built previously)
  • Ensure that the build-id is always set when writing ELF binaries and that it uses 20-byte output by default
  • Use zstd-compressed debug symbols by default for clang/lld/llvm-objcopy (for Enable zstd-compressed debug symbols in llvm + gcc #1347)

Addresses the LLVM part of #1346

@ermo ermo requested a review from joebonrichie December 12, 2024 22:33
@ermo
Copy link
Contributor

ermo commented Dec 12, 2024

@ReillyBrogan: Have you by any chance taken a look at whether it'd be possible to address this? #1346

@ReillyBrogan
Copy link
Contributor Author

@ReillyBrogan: Have you by any chance taken a look at whether it'd be possible to address this? #1346

LLVM doesn't use SHA1 for build-ids at all, and hasn't for quite some time. The Sha1 name is kept mainly for back-compat but always uses BLAKE3 instead but with everything after the first 20 bytes truncated (the length is what is kept for compat with tools that expect 20 bytes of output from --build-id=sha1).

Also LLVM already uses XXH3_64 when you use --build-id=fast, not really sure what else there is to do there? My patch only forces build-id=sha1 if --build-id is not present or when it is but without a specifier. If you directly use --build-id=fast then it will use that.

Changes:
- Re-enable building LLVM with PGO
- Enable perf support in the JIT engine
- Build MLIR-C library shared
- Install the lld man page
- Add "Solus $version-$release" to the Clang version
- Disable building libcxx and llvm benchmarks (these weren't installed but were still built previously)
- Ensure that the build-id is always set when writing ELF binaries and that it uses 20-byte output by default
- Use zstd-compressed debug symbols by default for clang/lld/llvm-objcopy

Signed-off-by: Reilly Brogan <[email protected]>
@ReillyBrogan
Copy link
Contributor Author

I added another patch that compresses debug symbols with zstd by default.

@ermo
Copy link
Contributor

ermo commented Dec 14, 2024

@ReillyBrogan: Have you by any chance taken a look at whether it'd be possible to address this? #1346

LLVM doesn't use SHA1 for build-ids at all, and hasn't for quite some time. The Sha1 name is kept mainly for back-compat but always uses BLAKE3 instead but with everything after the first 20 bytes truncated (the length is what is kept for compat with tools that expect 20 bytes of output from --build-id=sha1).

Also LLVM already uses XXH3_64 when you use --build-id=fast, not really sure what else there is to do there? My patch only forces build-id=sha1 if --build-id is not present or when it is but without a specifier. If you directly use --build-id=fast then it will use that.

The point is more to update our GCC and LLVM defaults to always use (the equivalent to) --build-id=fast I think?

If you know from experience that this is a Bad Idea™, then we should probably close the related issue...

@ReillyBrogan
Copy link
Contributor Author

Updating this from discussions on Matrix:

  • 64bits is a bit on the small side for the default hashing algorithm
  • It's unlikely that XXH3 is so much faster than BLAKE3 that it's worth going from 160 bits to 64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants