build 21.1.4 lto dbg #10
Conversation
WalkthroughActivated releasedbg/LTO matrix variants across Linux and macOS CI; switched Windows releasedbg to MT runtime with LTO enabled. xmake.lua pins LLVM to a commit, removes some subdirectory inclusions, and disables many optional LLVM/Clang components, narrowing the build scope. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Matrix as Job Matrix
participant Runner as Build Runner
rect rgb(245,250,255)
Note over GH,Matrix: Matrix expanded to include\nreleasedbg (lto:"n") and releasedbg (lto:"y")
end
GH->>Matrix: expand matrix combinations
Matrix->>Runner: dispatch jobs per combination
alt Windows releasedbg job
Runner->>Runner: run releasedbg (runtime=MT, LTO=y)
else Linux/macOS releasedbg jobs
Runner->>Runner: run releasedbg (LTO=n) or (LTO=y)
end
Runner->>GH: upload artifacts / report status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/linux.yml(1 hunks).github/workflows/macos.yml(1 hunks).github/workflows/windows.yml(1 hunks)xmake.lua(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). (8)
- GitHub Check: build (releasedbg, MT, y)
- GitHub Check: build (releasedbg, MD, n)
- GitHub Check: build (releasedbg, n)
- GitHub Check: build (releasedbg, y)
- GitHub Check: build (debug, n)
- GitHub Check: build (releasedbg, y)
- GitHub Check: build (debug, n)
- GitHub Check: build (releasedbg, n)
🔇 Additional comments (4)
.github/workflows/windows.yml (1)
16-18: LGTM! Matrix entry properly configured.The new
releasedbgbuild with MT runtime and LTO enabled complements the existing MD runtime configuration. The concurrency grouping and artifact naming will correctly differentiate these two variants..github/workflows/linux.yml (1)
17-18: LGTM! Consistent matrix expansion.The new
releasedbgconfiguration with LTO enabled aligns with the PR objectives and maintains consistency with the macOS and Windows workflow changes..github/workflows/macos.yml (1)
17-18: LGTM! Cross-platform consistency achieved.The macOS workflow now matches the Linux and Windows configurations with the addition of
releasedbgwith LTO enabled, providing consistent build options across all platforms.xmake.lua (1)
130-140: LGTM! Platform-specific configurations are appropriate.The platform-specific compiler and linker settings are well-configured:
- Windows:
clang-clfor MSVC ABI compatibility- Linux:
lldfor faster linking with LTO- macOS:
arm64architecture with appropriate Homebrew tool paths andlldlinkerThese settings align well with the PR's goal of supporting LTO builds across platforms.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
xmake.lua (1)
76-86: Remove duplicate configuration flags.The duplicate CMake flags identified in a previous review are still present:
LLVM_ENABLE_ZSTD=OFFappears on both lines 77 and 81LLVM_ENABLE_LIBXML2=OFFappears on both lines 78 and 85Apply this diff to remove the duplicates:
"-DLLVM_ENABLE_ZLIB=OFF", "-DLLVM_ENABLE_ZSTD=OFF", "-DLLVM_ENABLE_LIBXML2=OFF", "-DLLVM_ENABLE_BINDINGS=OFF", "-DLLVM_ENABLE_IDE=ON", - "-DLLVM_ENABLE_ZSTD=OFF", "-DLLVM_ENABLE_Z3_SOLVER=OFF", "-DLLVM_ENABLE_LIBEDIT=OFF", "-DLLVM_ENABLE_LIBPFM=OFF", - "-DLLVM_ENABLE_LIBXML2=OFF", "-DLLVM_ENABLE_OCAMLDOC=OFF",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
xmake.lua(2 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). (8)
- GitHub Check: build (releasedbg, MT, y)
- GitHub Check: build (releasedbg, n)
- GitHub Check: build (releasedbg, MD, n)
- GitHub Check: build (releasedbg, y)
- GitHub Check: build (debug, n)
- GitHub Check: build (releasedbg, y)
- GitHub Check: build (releasedbg, n)
- GitHub Check: build (debug, n)
🔇 Additional comments (2)
xmake.lua (2)
72-73: Clarify the removal of LTO tools while enabling LTO in the build.These lines remove the
ltotool subdirectory and implicit projects. However, line 129 enables LTO for the build (-DLLVM_ENABLE_LTO=...). While this is likely intentional (disabling the standalone LTO tools while using LTO compiler optimization), please confirm this doesn't impact the LTO build functionality that the PR objectives require.
102-102: Typo fixed - LGTM!The typo in
CLANG_TOOL_LIBCLANG_BUILDthat was flagged in a previous review has been corrected. The value is now properly set toOFFinstead of the incompleteO.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
xmake.lua (2)
28-28: Past comment still applies: Incorrect commit hash for LLVM 21.1.4.This issue was previously flagged and remains unresolved. The commit hash
222fc11f2b8f25f6a0f4976272ef1bb7bf49521ddoes not correspond to thellvmorg-21.1.4tag.
79-91: Past comment still applies: Remove duplicate configuration flags.The duplicate flags issue previously identified remains unresolved:
LLVM_ENABLE_ZSTD=OFFappears on both lines 80 and 84LLVM_ENABLE_LIBXML2=OFFappears on both lines 81 and 88
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
xmake.lua(2 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). (7)
- GitHub Check: build (releasedbg, y)
- GitHub Check: build (releasedbg, MD, n)
- GitHub Check: build (releasedbg, MT, y)
- GitHub Check: build (releasedbg, n)
- GitHub Check: build (releasedbg, y)
- GitHub Check: build (releasedbg, n)
- GitHub Check: build (debug, n)
🔇 Additional comments (1)
xmake.lua (1)
72-73: Verify that removing the LTO tool is intentional.Given the PR title mentions "lto", removing
add_llvm_tool_subdirectory(lto)might seem contradictory. However, this removes thellvm-ltocommand-line tool (which is separate from enabling LTO for the LLVM build itself via-DLLVM_ENABLE_LTO). Since line 97 already setsLLVM_BUILD_TOOLS=OFFand the goal appears to be a minimal build, this is likely intentional.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
xmake.lua (1)
76-105: Configuration flags appropriately narrow the build scope.The CMake flags correctly disable docs, tests, examples, benchmarks, and most tools, which aligns with building only the specific targets listed in lines 150-171. The approach of disabling unused components should reduce build time and resource usage.
Note: Line 96 has a commented-out flag. Consider either removing it or documenting why it's preserved.
If the commented flag is no longer needed, apply this diff:
"-DLLVM_INCLUDE_DOCS=OFF", "-DLLVM_BUILD_UTILS=OFF", "-DLLVM_BUILD_TOOLS=OFF", - -- "-DLLVM_INCLUDE_TOOLS=OFF", "-DCLANG_BUILD_TOOLS=OFF",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
xmake.lua(3 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). (8)
- GitHub Check: build (releasedbg, n)
- GitHub Check: build (releasedbg, y)
- GitHub Check: build (debug, n)
- GitHub Check: build (debug, n)
- GitHub Check: build (releasedbg, n)
- GitHub Check: build (releasedbg, y)
- GitHub Check: build (releasedbg, MT, y)
- GitHub Check: build (releasedbg, MD, n)
🔇 Additional comments (3)
xmake.lua (3)
17-17: LGTM – Adding third-party checkout.Including "third-party" in the sparse checkout is reasonable for the updated build configuration.
81-81: Verify LLVM_ENABLE_IDE=ON is intentional for CI builds.Enabling IDE support generates IDE-specific project files, which is typically unnecessary for CI/headless builds. Confirm this is intended (e.g., if developers will use these artifacts locally).
71-74: Subdirectory removals are isolated and align with narrowed build scope.The removal of
tool-template,ltotool subdirectory, and implicit projects is consistent withLLVM_BUILD_TOOLS=OFF(line 95). Verification confirms these removals have no other references in the codebase. Note that line 73 removes the LTO tool subdirectory, not LTO compilation mode itself (controlled by line 130).
Summary by CodeRabbit