Repository navigation
build: pre-build OOM-fragile MLIR objects - #29
Merged
Merged
Conversation
- MLIR dialect objects (LLVM, NVVM, ROCDL, SPIRV, Linalg, OpenACC, SparseTensor, RegisterAll) ran out of memory in Phase 1 on CI (-j4) and locally (-j8), costing a retry cycle each. - The MLIR entries in the old -j2 regex never matched anything new: Phase 1 had already built them before that phase ran. - Split Phase 1 into LLVM/Clang and MLIR phases. Between them, build `mlir-headers` at full parallelism, then the known OOM-fragile MLIR objects at -j2. Flang -j2 phase keeps only Flang/FIR dirs. - Share the -j2 object selection in `build_oom_objects`.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The phase ordering and target selection are consistent with the stated behavior, with no unresolved correctness issues found.
0 open findings
What changed in this PR
Splits the phased LLVM build to pre-build memory-intensive MLIR and Flang objects at reduced parallelism.
Changes:
- Separates LLVM/Clang and MLIR build phases.
- Adds a shared helper for building OOM-fragile objects at
-j2. - Preserves the existing single-pass target set.
| File | Description |
|---|---|
Dockerfile |
Adds OOM-aware MLIR pre-building and refactors phased target selection. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
zbeekman
commented
Oct 12, 2026
zbeekman
left a comment
Member
Author
There was a problem hiding this comment.
LGTM although that is a beast of a regex. We may need to think of a cleaner way to handle that.
Member
Author
|
I will try to test this again locally with a cold cache overnight tonight, but I'm not going to let that prevent merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Several MLIR dialect objects run out of memory in Phase 1 of the phased LLVM build, on CI (
-j4) and locally (-j8). Each one costs abuild-llvm.shretry cycle at reduced parallelism. The LLVM 23 local build needed 9 recoveries in Phase 1.The MLIR entries in the old
-j2regex did not help: Phase 1 had already built those objects before that phase ran.Fix
LLVM_CLANG_TARGETS) and MLIR (MLIR_TARGETS) phases.mlir-headersat full parallelism, then the 13 known OOM-fragile MLIR objects at-j2.-j2phase now matches only Flang and FIR object directories.-j2object selection into a sharedbuild_oom_objectshelper.PHASED_BUILD=false) builds the same targets as before.Compatibility
--build-arg LLVM_VERstill works. The version-specific logic is unchanged, andmlir-headersexists in LLVM 19, 22, and 23. The MLIR regex matches 13 objects on LLVM 22 and 23. The Flang regex matches 175 objects on LLVM 23 and 172 on LLVM 22. A pattern that matches nothing prints a warning and the build continues.Testing
salt-8cpu: Phase 2 built 13 MLIR objects and Phase 4 built 175 Flang objects at-j2, and the image exported.