chore(build): upgrade toolchain to clang 22 and LLVM prebuilt r2 - #550
Conversation
Rebuild the LLVM prebuilt (21.1.8) with clang 22 compiler toolchain. Key changes: - Bump all clang/lld/llvm-tools/compiler-rt pins from 20.1.8 to 22.1.8 - Bump gcc/gxx from 14.2.0 to 15.1.0 (build + test + cross-linux-arm64) - Bump clang-format from 21.1.7 to 22.1.8 - Use _LIBCPP_HAS_VENDOR_AVAILABILITY_ANNOTATIONS on macOS to fix __hash_memory undefined symbol errors against system libc++ - Pin MSVC toolset to 14.44 in build-llvm workflow to ensure prebuilt libs link with local VS 2022 Community (14.44) - Strip codegen options in parse_cc1_output to prevent CompilerInvocation::CreateFromArgs failures on unknown enum values from newer external drivers (e.g. -mframe-pointer=non-leaf-no-reserve)
CMake's Ninja generator on macOS uses Apple's libtool instead of ar for static libraries. Apple's libtool (Xcode 16.4, based on LLVM ~17) cannot read LLVM 22 bitcode (unknown attribute kind 102), breaking LTO builds. Use llvm-libtool-darwin if available from the toolchain; otherwise suppress CMAKE_LIBTOOL to fall back to llvm-ar via CMAKE_AR.
The ilammy/msvc-dev-cmd step defaults to x64, which sets LIB paths to x64 CRT libraries. ARM64 cross-compilation jobs then fail with "machine type x64 conflicts with arm64". Use amd64_arm64 arch for Windows ARM64 cross-builds.
ilammy/msvc-dev-cmd with amd64_arm64 arch sets LIB paths to ARM64 directories only, but the native tools build step needs x64 libraries. The toolset pin is only needed for native x64 builds (ensures prebuilt .lib compatibility with local MSVC 14.44). ARM64 cross-builds use the runner's default VS environment.
ilammy/msvc-dev-cmd with toolset 14.44 on VS 2025 (version 18) sets up incomplete include paths missing ATL headers, causing 'atlbase.h' not found in LLVM's DIA SDK build. Setting VCToolsVersion env var instead preserves the runner's default environment (with ATL) while pinning the MSVC toolset version for cmake detection.
- Add LLVM_ENABLE_DIA_SDK=OFF to build-llvm.py — ATL headers are not available on all Windows CI toolsets, and clice doesn't need DIA. - Replace VCToolsVersion env var with ilammy/msvc-dev-cmd action for reliable MSVC toolset pinning (14.42).
Cherry-pick d1a5331 (PR #160804) which replaces illegal std::less and std::equal_to specializations in RDFRegisters with proper custom types. libc++ 22 enforces is_empty on comparators used in std::set/map, and the old specialization had a pointer member.
Clone clice-io/clice-llvm and apply version-matched patches to the upstream LLVM source. For 21.1.8 this fixes the illegal std::less specialization that triggers libc++ 22's static_assert on macOS.
Normal builds use clice-llvm main. To reproduce a previous build, pass patch_ref=<release-tag> (e.g., 21.1.8+r2) — the workflow clones clice-llvm at that tag, falling back to main if the tag doesn't exist. release-llvm tags clice-llvm when publishing, freezing the exact patches used.
- Fix patch apply: git -C .llvm needs absolute path since the patch file is outside .llvm - Remove MSVC toolset pin: windows-2025 runners use VS 2026 which doesn't have 14.42. MSVC version pinning needs a different approach (to be addressed separately).
- MSVC: use ilammy/msvc-dev-cmd for native x64 only (toolset 14.44). ARM64 cross skips pin (ilammy conflicts with native tools build). VCToolsVersion env var doesn't work with clang-cl. - macOS: add _LIBCPP_HAS_VENDOR_AVAILABILITY_ANNOTATIONS=1 to native tools CXX flags too (not just main build).
Use clang-cl's /vctoolsversion flag instead of ilammy/msvc-dev-cmd. This pins the toolset for BOTH native and cross builds without polluting the global environment — native tools build is unaffected. Also ensures native tools build on macOS gets the availability annotation define.
Replace /vctoolsversion flag and ilammy action with direct vcvarsall.bat calls. This correctly sets INCLUDE/LIB/PATH for the pinned toolset (14.44) and handles cross-compilation by calling vcvarsall with the right arch at the right time: - Native tools: amd64 (always host arch) - Main build: amd64 (native) or amd64_arm64 (cross)
vcvarsall.bat called twice in one process conflicts (x64 env leaks into ARM64 cross build). Revert to the proven approach: - Native x64: ilammy/msvc-dev-cmd with toolset 14.44 - ARM64 cross: no MSVC pin (uses default toolset) ARM64 cross prebuilt with default toolset is acceptable — only CI uses it, local dev machines don't cross-compile to ARM64 Windows.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request updates LLVM toolchain versions and build workflows, adds macOS-specific compiler and archive handling, applies versioned LLVM patches, changes Windows runners, and filters code-generation options from parsed cc1 arguments. ChangesLLVM build and toolchain
cc1 argument filtering
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-llvm.yml:
- Around line 10-14: Update the workflow input `patch_ref` to default to the
immutable release ref matching LLVM 21.1.8, such as `21.1.8+r2`, instead of
`main`. In the patch-fetch logic, only fall back to `main` when the requested
release ref is confirmed missing; propagate or fail on transient clone/fetch
errors so they cannot silently change the patch set.
- Around line 166-167: Remove direct GitHub Actions interpolation of
llvm_version and patch_ref from the shell in the relevant build and clone steps.
Expose both inputs through step-level env variables, assign VERSION and REF from
quoted LLVM_VERSION and PATCH_REF values, and validate llvm_version before using
it to construct branch or path arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 520e0328-27c9-4de7-aa69-108db8b728b2
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.github/workflows/build-llvm.ymlCMakeLists.txtcmake/package.cmakecmake/toolchain.cmakepixi.tomlscripts/build-llvm.pysrc/command/argument_parser.cppsrc/command/toolchain.cpptests/unit/command/toolchain_tests.cpp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a680ecea9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kotatsu's top-level CMakeLists adds -D_LIBCPP_DISABLE_AVAILABILITY globally. On macOS with conda libc++ >= 21 that macro makes the libc++ headers emit extern references to dylib-only symbols (__hash_memory, llvm-project#77653) that the macOS system libc++ lacks, breaking the x64 cross link (undefined symbol in libkota_ipc.a) and poisoning shipped binaries. Directory-level COMPILE_OPTIONS are copied into each target at creation time, so walk every target under the kotatsu source tree and strip the flag. Temporary until the flag is removed upstream in kotatsu.
These _LIBCPP_HAS_VENDOR_AVAILABILITY_ANNOTATIONS=1 defines were added on a false premise: that conda's libc++ headers ship without vendor availability annotations. They ship with the annotations enabled by default, so the defines were no-ops. Remove them from the APPLE block in CMakeLists.txt and the two darwin sites in build-llvm.py.
conda-forge clang 22's default config files inject env -L/-rpath at link time, binding binaries to conda's @rpath libc++ that doesn't exist outside the build machine. Passing --no-default-config restores system-libc++ linking, which is safe thanks to availability annotations.
No macOS package sets the cleared vars anymore (conda moved to cfg-based packaging) and the cross-macos-x64 pixi env it belonged to is unused by CI — the macos-x64 cross job uses the default env.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CMakeLists.txt`:
- Around line 29-39: The Apple --no-default-config settings are applied after
project() compiler detection, allowing conda defaults to be cached prematurely.
Move these settings before project() by initializing
CMAKE_CXX_COMPILE_OPTIONS_INIT and the executable/shared linker flag init
variables, guarded by CLICE_USE_LIBCXX, and align them with the existing
CLICE_CXX_FLAGS, CLICE_EXE_LINKER_FLAGS, and CLICE_SHARED_LINKER_FLAGS
overrides.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5afe2e04-f134-4019-b09e-a20813817e3d
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
CMakeLists.txtpixi.tomlscripts/activate_cross_macos.sh
💤 Files with no reviewable changes (2)
- scripts/activate_cross_macos.sh
- pixi.toml
The macOS --no-default-config compiler/linker flags lived in CMakeLists.txt, so only clice's own build saw them. Move them to toolchain.cmake as CMAKE_*_FLAGS_INIT so every build using the toolchain file inherits them — including build-llvm.py's main LLVM build (prebuilt r3), which now gets the flag for free instead of plumbing it separately.
The prebuilt LLVM ASan dylibs linked in Debug reference conda's @rpath libc++ with rpaths baked for the machine that built the prebuilt, so Debug binaries fail to load elsewhere. Add an env libc++ rpath via CMAKE_*_LINKER_FLAGS_DEBUG_INIT so Debug resolves libc++ from the build env. Debug binaries are CI-internal only. Temporary until the prebuilt is respun with --no-default-config, after which its dylibs will link the system libc++.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56707a1344
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two clang 22 modules changes need source-level fixes: - Reduced BMI is now the default; its merged lookup table has a known bug (llvm/llvm-project#205361, fixed in clang 23 by PR #186337) that breaks concept-constrained implicit conversions and causes std::to_string ambiguity. Add -fno-modules-reduced-bmi until clang 23. - Unscoped enum values from the GMF are no longer visible to importers (spec-conforming fix, llvm/llvm-project#131058). Explicitly export CXAvailabilityKind and RefQualifierKind from the clang wrapper module. The pixi toolchain pins that originally rode along with this commit landed separately on main via #550.
Two clang 22 modules changes need source-level fixes: - Reduced BMI is now the default; its merged lookup table has a known bug (llvm/llvm-project#205361, fixed in clang 23 by PR #186337) that breaks concept-constrained implicit conversions and causes std::to_string ambiguity. Add -fno-modules-reduced-bmi until clang 23. - Unscoped enum values from the GMF are no longer visible to importers (spec-conforming fix, llvm/llvm-project#131058). Explicitly export CXAvailabilityKind and RefQualifierKind from the clang wrapper module. The pixi toolchain pins that originally rode along with this commit landed separately on main via #550.
Summary
Toolchain upgrade:
macOS dynamic-linking fixes — libc++ ≥ 21 moved string hashing into the dylib as
__hash_memory(llvm/llvm-project#77653), which surfaced two latent issues in our setup:_LIBCPP_DISABLE_AVAILABILITYfrom the kotatsu targets. With libc++ ≥ 21 headers that macro makes the headers emit references to dylib-only symbols the macOS system libc++ does not ship, which broke the x64 cross link and made binaries depend on conda's libc++ at runtime. Temporary workaround until the flag is removed upstream in kotatsu.--no-default-configto clang on Apple, via the toolchain file so the LLVM prebuilt builds inherit it. conda-forge clang 22's bundled config files inject-L/-rpathpointing into the conda env at link time, binding binaries to@rpath/libc++.1.dylibwith an rpath valid only on the build machine — the VSCode E2E job caught them failing to load anywhere else. With config files disabled, binaries link the system libc++, gated safely by availability annotations.Test plan
/usr/lib/libc++.1.dylibwith no conda env rpath (verified viaotool -L)