Repository navigation
feat(ci): Add build_stages input to filter CI build graph - #7600
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
7ebe548 to
c388f0d
Compare
ScottTodd
left a comment
There was a problem hiding this comment.
skip_stages on its own LGTM, I don't think the validate_artifact_structure change should be needed though.
BTW, I added similar "skip arbitrary stages" to another CI system before, see https://iree.dev/developers/general/contributing/#ci-behavior-manipulation and the code like if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'test_android') in https://github.com/iree-org/iree/blob/main/.github/workflows/pkgci.yml
c388f0d to
effc623
Compare
2a9404e to
fcd8830
Compare
geomin12
left a comment
There was a problem hiding this comment.
the whole purpose of multi-arch is to re-use or re-build all of ROCm, so we can make sure downstream packaging (pytorch, python packages, tarball packaging) is not impact (so release packages don't break). if we skip, we won't be able to test downstream packages
i did exact work here: #6709 but aligned with team to understand the purpose of multi-arch and its importance
Thanks for the context on multi-arch's purpose. I agree that for release builds and others we need the full pipeline to validate downstream packaging. The build_stages input is intended for specialized CI configurations, not to replace the default multi-arch flow. The primary use case is #7202: host-asan This allows callers to create focused ASAN presubmit workflows that:
The default behavior remains unchanged — callers that don't set build_stages still get the full multi-arch pipeline. |
fcd8830 to
0009a50
Compare
We want both:
choosing between those will be "defcon" levels documented at https://github.com/ROCm/TheRock/blob/main/docs/rfcs/RFC0013-Consumer-Based-Test-Selection.md#background-gating-levels see also the diagrams I posted originally at #3343 (comment) in particular, case 3 looks like it could benefit from skipping entire build stages
|
geomin12
left a comment
There was a problem hiding this comment.
lgtm, based on Scott's diagram 3!
| build_stages: | ||
| description: >- | ||
| Comma-separated allowlist of build stages to run, e.g. | ||
| "compiler-runtime,runtime-tests". Every other stage is skipped | ||
| outright, with nothing built and no artifacts copied. Empty (the | ||
| default) builds every stage. | ||
| type: string | ||
| default: "" |
There was a problem hiding this comment.
Let's expose this to https://github.com/ROCm/TheRock/blob/main/.github/workflows/multi_arch_ci.yml for workflow_dispatch also, as that will
- Let us test the feature itself without needing to edit other workflows, see https://github.com/ROCm/TheRock/blob/main/TESTING.md#github-actions-workflows---validation-methods
If a workflow does not run on pull_request events, test manually with workflow_dispatch (https://docs.github.com/en/actions/how-tos/manage-workflow-runs/manually-run-a-workflow) and link the test runs in the pull request description.
- Use this for https://github.com/ROCm/TheRock/blob/main/docs/development/ci_behavior_manipulation.md#workflow-dispatch. I'd like to use this to test just building
compiler-runtime(or just the LLVM subproject actually, so a "build_subprojects" or "enable_subprojects" input that gets mapped to build stages and test subprojects could also be useful too)
I'm also looking over the current integration of these workflows into https://github.com/ROCm/rocm-libraries and https://github.com/ROCm/rocm-systems and I think we should consider changing https://github.com/ROCm/rocm-libraries/blob/develop/.github/workflows/therock-multi-arch-ci.yml from
jobs:
ci-env:
...
configure:
...
resolve_therock_ref:
...
setup:
uses: ROCm/TheRock/.github/workflows/setup_multi_arch.yml@eb36dd0822a5e9a53c47fae4abad0b4a978fd6f5 # 08/21/2026
...
linux_build_and_test:
uses: ROCm/TheRock/.github/workflows/multi_arch_ci_linux.yml@eb36dd0822a5e9a53c47fae4abad0b4a978fd6f5 # 08/21/2026
...
windows_build_and_test:
uses: ROCm/TheRock/.github/workflows/multi_arch_ci_windows.yml@eb36dd0822a5e9a53c47fae4abad0b4a978fd6f5 # 08/21/2026
...
multi_arch_ci_summary:
...to:
jobs:
multi_arch_ci:
uses: ROCm/TheRock/.github/workflows/multi_arch_ci.yml@eb36dd0822a5e9a53c47fae4abad0b4a978fd6f5 # 08/21/2026that would be less code to maintain in each repository, though it will require making all customized behaviors available to multi_arch_ci.yml (which will also let us centralize logging, testing, monitoring, etc.)
There was a problem hiding this comment.
I agree with the goal on having a singular uses call. That was my desired design for the original de-dupe effort on the legacy CI system.
There was a problem hiding this comment.
Exposed "build_stages" to multi_arch_ci.yml
https://github.com/ROCm/TheRock/actions/runs/33016801269
838066b to
c8c8d18
Compare
|
@ScottTodd Can we merge this if it looks good? If any non-blocking changes are required, can we address them in a separate PR? |
| # Filter test labels based on build_stages when a partial build is requested. | ||
| # Tests that require artifacts from skipped stages cannot run. | ||
| linux_test_labels = ci_inputs.linux_test_labels | ||
| windows_test_labels = ci_inputs.windows_test_labels | ||
| allowed_labels = _get_allowed_test_labels_for_stages(ci_inputs.build_stages) | ||
| if allowed_labels is not None: | ||
| # Partial build: filter to only labels that can run with available stages. | ||
| # If caller specified labels, intersect with allowed; otherwise use allowed. | ||
| if linux_test_labels: | ||
| filtered = [ | ||
| lbl | ||
| for lbl in linux_test_labels | ||
| if lbl.replace("test:", "") in allowed_labels | ||
| ] | ||
| if filtered != linux_test_labels: | ||
| print( | ||
| f" Filtered linux_test_labels for partial build: " | ||
| f"{linux_test_labels} -> {filtered}" | ||
| ) | ||
| linux_test_labels = filtered | ||
| else: | ||
| # No explicit labels: auto-select based on stages | ||
| linux_test_labels = [f"test:{lbl}" for lbl in allowed_labels] | ||
| print( | ||
| f" Auto-selected linux_test_labels for partial build: {linux_test_labels}" | ||
| ) | ||
|
|
||
| if windows_test_labels: | ||
| filtered = [ | ||
| lbl | ||
| for lbl in windows_test_labels | ||
| if lbl.replace("test:", "") in allowed_labels | ||
| ] | ||
| if filtered != windows_test_labels: | ||
| print( | ||
| f" Filtered windows_test_labels for partial build: " | ||
| f"{windows_test_labels} -> {filtered}" | ||
| ) | ||
| windows_test_labels = filtered |
There was a problem hiding this comment.
This label processing code needs close review.
All CI configuration code should fit within the existing stages documented at the very top of this file:
TheRock/build_tools/github_actions/configure_multi_arch_ci.py
Lines 7 to 15 in c3621af
TheRock/build_tools/github_actions/configure_multi_arch_ci.py
Lines 242 to 252 in c3621af
Also note that labels are only one way that tests are selected, typically as part of opting in to running a known list of jobs (https://github.com/ROCm/TheRock/blob/main/docs/development/ci_behavior_manipulation.md#pull-request). If we compute the stages to skip dynamically (not just from fixed workflow inputs), we'll need to align both the build and test jobs to know about that. I don't think that's necessary yet here though.
If only handling fixed labels in workflow files or manual inputs, we could also treat mismatched labels as errors or just proceed and let test jobs fail. Fail fast behavior is preferred over silently falling back.
What behavior are you looking for out of the code here? How do you intend on using this? The PR description is pretty sparse on details so I'm left guessing.
There was a problem hiding this comment.
- Fail-fast behavior: Changed from silent filtering to raising ValueError if test labels require stages not in build_stages. Incompatible labels now fail
immediately during setup rather than at test runtime. - Code organization: Moved validation into CIInputs.validate(), called from from_environ() in step 1 "Parse Inputs" where labels are parsed. Removed the
post-processing from configure().
| # Maps build stages to the test labels that can run with artifacts from those | ||
| # stages. Used to auto-filter tests when build_stages narrows the build graph. | ||
| # Test labels not listed here require a full build. | ||
| STAGE_TO_TEST_LABELS: dict[str, list[str]] = { | ||
| "compiler-runtime": ["kfdtest"], | ||
| "runtime-tests": ["hip-tests", "rocrtst"], | ||
| "math-libs": [ | ||
| "rocblas", | ||
| "hipblas", | ||
| "hipblaslt", | ||
| "rocfft", | ||
| "hipfft", | ||
| "rocrand", | ||
| "hiprand", | ||
| "rocsparse", | ||
| "hipsparse", | ||
| "rocsolver", | ||
| "hipsolver", | ||
| "rocprim", | ||
| "hipcub", | ||
| "rocthrust", | ||
| "rocalution", | ||
| "rocwmma", | ||
| "hiptensor", | ||
| "composable-kernel", | ||
| ], | ||
| "ml-libs": ["miopen", "hipdnn", "miopenprovider", "hipblasltprovider"], | ||
| "comm-libs": ["rccl", "rocshmem"], | ||
| "storage-libs": ["hipfile"], | ||
| "profiler-apps": ["rocprofiler-systems", "rocprofiler-compute"], | ||
| "cv-libs": ["rpp"], | ||
| "media-libs": ["rocdecode", "rocjpeg"], | ||
| "debug-tools": ["rocgdb"], | ||
| } |
There was a problem hiding this comment.
This build_tools/github_actions/configure_multi_arch_ci.py script is currently fully generic - it does not know anything about specific build stages or subproject names. I do think it should know about the github labels it handles (https://github.com/ROCm/TheRock/blob/main/docs/development/ci_behavior_manipulation.md#pull-request), but maybe we can find a separate file to know about these mappings, like https://github.com/ROCm/TheRock/blob/main/build_tools/github_actions/fetch_test_configurations.py?
WDYT @geomin12 ?
c8c8d18 to
2aadb2a
Compare
Add a build_stages allowlist input to setup_multi_arch.yml that lets
callers specify which stages to build. Stages outside the allowlist
are skipped entirely (no build, no artifact copy). This enables
focused CI runs that only build specific pipeline stages.
- Add build_stages input to setup_multi_arch.yml
- Add skip_stages input to multi_arch_build_portable_linux.yml and
multi_arch_build_windows.yml
- Add _resolve_skipped_stages() to convert allowlist to skip list
- Pass skip_stages through multi_arch_ci_{linux,windows}.yml
- Auto-disable validate_artifact_structure for partial builds
…ages Per review feedback, artifact validation should work on partial builds and provides meaningful results for subsets of artifacts. Removed the auto-disable logic that skipped validation when build_stages was set. Artifact validation now always runs regardless of build_stages.
1. Expose build_stages to multi_arch_ci.yml for workflow_dispatch testing
2. Disable package builds (python, pytorch, jax, native) when stages are
skipped - packages require complete artifact sets. Prebuilt/reused
stages are OK since their artifacts are copied from baseline.
3. Auto-filter test labels based on build_stages - tests requiring
artifacts from skipped stages cannot run
4. Fix is_asan check to cover both "asan" and "host-asan" variants
Address review feedback:
- Add CIInputs.validate() method with fail-fast behavior: raises
ValueError if test labels require stages not in build_stages
- Call validate() from from_environ() so validation happens in
step 1 "Parse Inputs" where labels are parsed
- Remove post-processing from configure() function
Fail-fast is preferred over silently filtering incompatible labels.
b7c9dc4 to
14a2f51
Compare
When build_stages limits the build and no test labels are provided, auto-select compatible labels based on STAGE_TO_TEST_LABELS mapping. This ensures only tests that can run with the available stages are selected. Behavior in from_environ() (Parse Inputs step): - Labels provided: fail-fast if they require stages not in build_stages - No labels provided: auto-select compatible labels for the stages This addresses the issue where partial builds with no explicit test labels would attempt all tests, causing failures for missing artifacts.
|
@ScottTodd FYI, Added auto-selection of test labels when none are provided.
Latest run passed with only kfdtest triggered for compiler-runtime stage. : |
ScottTodd
left a comment
There was a problem hiding this comment.
Ok, looks good. As we get more sophisticated with enabling/disabling/reusing builds and tests I think we'll want to do some deeper refactoring here, to keep build_tools/github_actions/configure_multi_arch_ci.py more streamlined. It's starting to reach a complexity level that is difficult to follow.
) ## Motivation The multi-arch CI (`therock-multi-arch-ci.yml`) currently rebuilds **every** TheRock build stage on **every** rocm-systems PR, regardless of how small the change is. This is because rocm-systems is represented as a single `source_set` in TheRock's `BUILD_TOPOLOGY.toml`, so any change maps to the full stage graph and nothing can be narrowed. The result is slow per-PR turnaround and a scaling bottleneck as more projects land in the monorepo. The pre-multi-arch CI only built the components a PR actually touched. This PR restores that behavior for the multi-arch pipeline: it builds **only the stages a PR needs** and skips the rest outright (no build, no artifact copy). This is a pure per-PR speed-up; it does not change what a full/nightly build produces. ## Technical Details This builds on **ROCm/TheRock#7600** ("feat(ci): Add `build_stages` input to filter CI build graph"), now merged, which adds a `build_stages` **allowlist** input to TheRock's `setup_multi_arch.yml`. When set, TheRock builds only the listed stages and skips every other stage outright; it also validates unknown stage names and auto-scopes test labels to the allowlisted stages. rocm-systems just needs to compute and pass the allowlist. Changes in this PR (rocm-systems only): - **New `.github/scripts/compute_build_stages.py`.** Maps the changed projects (already produced by TheRock's `configure_external_repo_ci.py`) to the minimal allowlist of build stages required to build them, using TheRock's build topology as the single source of truth (`BuildTopology.get_stages_for_projects`, which walks the upstream build-dependency closure). The topology module is imported read-only from the TheRock checkout, so this script owns no dependency math and cannot drift from the actual build graph. - **`therock-multi-arch-ci.yml` wiring.** - The `configure` job's repo sparse-checkout now also fetches the new script; the TheRock sparse-checkout is broadened to include `build_tools/_therock_utils` (cone mode also pulls the root `BUILD_TOPOLOGY.toml` and `build_tools/project_mappings.json`). - A new "Compute build-stage allowlist" step emits `build_stages`. - The `setup` job passes `build_stages: ${{ needs.configure.outputs.build_stages }}` to `setup_multi_arch.yml`. **Dependency / rollout.** This PR does **not** change the pinned TheRock ref; the `build_stages` input is consumed only once the pinned ref includes #7600, which is being bumped in a separate PR. Until that lands, `build_stages` is an inert no-op (unknown inputs are ignored), so this change is safe to merge independently and activates automatically when the ref is updated. Keep the configure-checkout ref and the `setup`/`multi_arch_ci_*` refs in sync so the topology used for selection matches what is built. **Fail-safe (empty allowlist ⇒ build all).** The script conservatively builds everything whenever the change set cannot be safely narrowed: - CI-infra changes (`run_all_tests` true — workflow/scripts/`repos-config.json`); - no changed projects; - a fan-out project whose ABI ripples into consumers (`clr`, `hip`, `hipother`, `hip-tests`, `rocr-runtime`, `amdsmi`); - any unrecognized/unmapped project (guards the mixed known/unknown case, since `get_stages_for_projects` silently ignores names it cannot resolve); - any topology-load error. **Safety.** `get_stages_for_projects` always returns the full upstream closure, so no built stage ever depends on a skipped stage. The trade-off while active: skipped stages produce no artifacts, so downstream jobs see only the built subset. This is intentional and temporary; see the tracking issue below. Relevant links: - TheRock build_stages input: ROCm/TheRock#7600 - TheRock stage/topology API: `build_tools/_therock_utils/build_topology.py` (`get_stages_for_projects`), `build_tools/project_mappings.json` ## Issue Tracking ISSUE ID: ROCm/rocm-libraries#11195 ## Test Plan - Unit tests (`.github/scripts/tests/compute_build_stages_test.py`) covering: project-path normalization, the `run_all_tests` short-circuit, fan-out policy, mixed known/unknown projects, the "all stages required" case, and topology-load failure. The topology is stubbed so the tests need no TheRock checkout. - Ran `compute_build_stages.py` against a real TheRock checkout pinned to the #7600 merge commit for representative changed-project sets. - Validated the workflow parses as YAML and the patch applies cleanly on `develop`. ## Test Result - 10/10 unit tests pass; `black` clean; workflow YAML valid. - Representative allowlists against the pinned TheRock topology: - `projects/rdc` → `compiler-runtime,dctools-core` - `projects/rocshmem` → `comm-libs,compiler-runtime` - `projects/rocprofiler-systems` → `compiler-runtime,profiler-apps` - `projects/hip` (fan-out), unknown project, empty, and `run_all_tests=true` → empty (build all). ISSUE ID: ROCm/TheRock#3343 ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
Validate ROCm/TheRock#7780 on top of current main (which already carries ROCm/TheRock#7602 and ROCm/TheRock#7600) without the sanity-check fixes still in review as ROCm/TheRock#8100. Co-authored-by: Cursor <cursoragent@cursor.com>

Add build_stages allowlist input to filter CI build graph
Use case: Partial builds in downstream repos (rocm-systems) that only
need specific stages. For example, rocm-systems only needs
compiler-runtime - it doesn't need math-libs, ml-libs, comm-libs, etc.
With build_stages, the workflow skips unneeded stages, reducing CI time.
Add a build_stages allowlist input to setup_multi_arch.yml that lets
callers specify which stages to build. Stages outside the allowlist
are skipped entirely (no build, no artifact copy).
Changes:
multi_arch_build_windows.yml
test labels require stages not in build_stages
complete artifacts; prebuilt/reused stages are OK)
ISSUE ID: #7202
Test job: https://github.com/ROCm/TheRock/actions/runs/33142065664