Skip to content

Conversation

@burgholzer
Copy link
Member

Testing in mqt-core has revealed that using LTO leads to problems when compiling with different compilers than the ones being used to compile the static libraries.

This PR disables the LTO option for macOS arm64, which was the only setting where it was enabled.

This also directly prepares a release, which should allow us to test the actions capabilities of picking the latest released version of a particular LLVM tag.

@burgholzer burgholzer self-assigned this Dec 6, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 6, 2025

📝 Walkthrough

Walkthrough

This pull request disables Link Time Optimization (LTO) on macOS arm64 builds. The CHANGELOG.md documents this change under release 2025.12.06, while the build script implements it by setting ENABLE_LTO to OFF for arm64/aarch64 targets, changing from THIN.

Changes

Cohort / File(s) Change Summary
macOS LTO Configuration
CHANGELOG.md, scripts/toolchain/macos/build.sh
Added changelog entry for 2025.12.06 documenting LTO disablement on macOS arm64; modified build script to set ENABLE_LTO=OFF for arm64/aarch64 hosts (previously THIN)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5–10 minutes

  • Verify that ENABLE_LTO=OFF is the correct setting for arm64 compatibility
  • Confirm LLVM tag llvmorg-21.1.7 alignment with the build configuration
  • Check that x86_64 path remains unchanged and unaffected

Poem

🐰 Hop, hop! The arm64 builds now run so free,
No LTO tangles in the machinery,
From THIN to OFF, a lighter load,
Swift and steady down the macOS road! 🚀

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: disabling LTO on macOS arm64 builds, which matches the primary modification in the changeset.
Description check ✅ Passed The description is directly related to the changeset, explaining the rationale for disabling LTO and mentioning the release preparation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch disable-lto

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26ad2fb and c80c5fd.

📒 Files selected for processing (2)
  • CHANGELOG.md (2 hunks)
  • scripts/toolchain/macos/build.sh (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: macos / build (macos-15-intel)
  • GitHub Check: macos / build (macos-15)
🔇 Additional comments (4)
scripts/toolchain/macos/build.sh (2)

49-51: ✅ LTO configuration change is correct and consistent.

The change properly disables LTO for arm64/aarch64 hosts (line 51), bringing them into parity with x86_64 (line 54). Both architectures now have ENABLE_LTO=OFF, which aligns with the PR objective and addresses the compiler compatibility issue noted in the PR description.


89-89: ✅ CMake variable usage is correct.

The ENABLE_LTO variable is properly quoted in the cmake invocation, ensuring safe substitution regardless of the assigned value.

CHANGELOG.md (2)

11-19: ✅ Changelog entry is well-formatted and complete.

The new 2025.12.06 release section follows the established format, includes the LLVM distribution tag, and documents the LTO change with proper attribution and PR reference. The emoji and markdown syntax are consistent with existing entries.


46-47: ✅ Changelog link references are properly configured.

The unreleased comparison link and new release tag link are correctly updated. The PR #29 link is properly added and follows the existing link format convention.

Also applies to: 53-53


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@burgholzer burgholzer added fix Fixes something that is not working continuous integration CI-related changes labels Dec 6, 2025
@burgholzer burgholzer merged commit a55b2f8 into main Dec 6, 2025
13 checks passed
@burgholzer burgholzer deleted the disable-lto branch December 6, 2025 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

continuous integration CI-related changes fix Fixes something that is not working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants