Repository navigation
ci: use host-ASan for submodule bumps and add an ASan-debug label #8716
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c0a9d58
4cec0da
0ed53fb
b1d4f92
49231ca
7dc63de
be9bbc1
a6efb4c
0eeb302
2841b40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -366,18 +366,25 @@ def test_none_changed_files_skips_path_filter(self, mock_filter): | |
| def test_asan_pr_without_label_skips(self): | ||
| """ASAN PR without enabling label skips CI.""" | ||
| inputs = self._inputs(build_variant="asan", pr_labels=[]) | ||
| git = cm.GitContext( | ||
| changed_files=["CMakeLists.txt", "build_tools/script.py"], | ||
| ) | ||
| self.assertTrue(cm.should_skip_ci(inputs, git)) | ||
| for changed_files in ( | ||
| ["CMakeLists.txt", "build_tools/script.py"], | ||
| ["compiler/amd-llvm"], | ||
| ): | ||
| with self.subTest(changed_files=changed_files): | ||
| git = cm.GitContext( | ||
| changed_files=changed_files, submodule_paths=["compiler/amd-llvm"] | ||
| ) | ||
| self.assertTrue(cm.should_skip_ci(inputs, git)) | ||
|
Comment on lines
-369
to
+377
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Subtest is not useful here. All that matters is that CI is skipped when there are code changes and no asan label. We don't need to separately check |
||
|
|
||
| def test_asan_pr_with_ci_asan_label_runs(self): | ||
| """ASAN PR with ci:asan label runs CI.""" | ||
| inputs = self._inputs(build_variant="asan", pr_labels=["ci:asan"]) | ||
| def test_asan_pr_with_enabling_label_runs(self): | ||
| """Each sanitizer label opts a PR into ASAN CI.""" | ||
| git = cm.GitContext( | ||
| changed_files=["CMakeLists.txt", "build_tools/script.py"], | ||
| ) | ||
| self.assertFalse(cm.should_skip_ci(inputs, git)) | ||
| for label in ("ci:asan-debug", "ci:asan", "ci:host-asan"): | ||
| with self.subTest(label=label): | ||
| inputs = self._inputs(build_variant="asan", pr_labels=[label]) | ||
| self.assertFalse(cm.should_skip_ci(inputs, git)) | ||
|
|
||
| def test_asan_non_pr_runs(self): | ||
| """ASAN on schedule/push runs regardless of labels.""" | ||
|
|
@@ -1832,6 +1839,39 @@ def test_variant_filters_by_trigger(self): | |
| build_variant_cmake_preset = result.linux.build_variant_cmake_preset | ||
| self.assertEqual(build_variant_cmake_preset, expected_variant) | ||
|
|
||
| def test_asan_label_selection(self): | ||
| """PR labels select the sanitizer variant in priority order.""" | ||
| cases = ( | ||
| (["ci:host-asan"], "host-asan"), | ||
| (["ci:asan"], "asan"), | ||
| (["ci:asan-debug"], "asan-debug"), | ||
| (["ci:asan", "ci:host-asan"], "asan"), | ||
| (["ci:asan-debug", "ci:asan", "ci:host-asan"], "asan-debug"), | ||
| ) | ||
| targets = cm.TargetSelection(linux_families=["gfx94x", "gfx950"]) | ||
| for labels, expected_variant in cases: | ||
| with self.subTest(labels=labels): | ||
| result = cm.expand_build_configs( | ||
| ci_inputs=self._inputs( | ||
| event_name="pull_request", | ||
| build_variant="asan", | ||
| pr_labels=labels, | ||
| ), | ||
| git_context=cm.GitContext(), | ||
| targets=targets, | ||
| jobs=_jobs(), | ||
| ) | ||
| linux = result.linux | ||
| self.assertEqual(linux.build_variant_label, expected_variant) | ||
| self.assertEqual( | ||
| linux.build_variant_cmake_preset, | ||
| f"linux-release-{expected_variant}", | ||
| ) | ||
| self.assertEqual( | ||
| [entry["test-runs-on"] for entry in linux.per_family_info], | ||
| ["", ""], | ||
| ) | ||
|
|
||
| def test_push_asan_excludes_families_without_host_asan_support(self): | ||
| """Push ASAN: gfx110x only supports release, so it must be excluded. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,8 +23,10 @@ The following labels may be added to a pull request to modify CI behavior: | |
| | -------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `ci:skip` | Skip all builds and tests | | ||
| | `ci:run-all-archs` | Build and test all possible architectures | | ||
| | `ci:asan` | Enable ASAN CI builds and tests. ASAN CI is skipped by default on PRs unless this label is present. | | ||
| | `ci:host-asan` | Alias for `ci:asan`. Enable ASAN CI builds and tests. | | ||
| | `ci:asan-debug` | Select full ASan builds with debug information. GPU tests remain limited to nightly/manual runs. Takes precedence over all other sanitizer labels. | | ||
| | `ci:asan` | Select full ASan builds. GPU tests remain limited to nightly/manual runs. Takes precedence over host-ASan labels. | | ||
| | `ci:host-asan-debug` | Select host-ASan builds with debug information. GPU tests remain limited to nightly/manual runs. Takes precedence over `ci:host-asan`. | | ||
| | `ci:host-asan` | Select host-ASan builds. GPU tests remain limited to nightly/manual runs. | | ||
|
sa-faizal marked this conversation as resolved.
Resolved
|
||
| | `ci:run-multi-gpu` | Opt-in to running multi-GPU tests. | | ||
| | `ci:cpu-test-only` | Run only CPU tests (skip GPU tests). Useful for changes that don't require GPU testing. | | ||
| | `ci:platform:linux` | Run CI only on Linux (skip Windows builds/tests) | | ||
|
|
@@ -76,6 +78,25 @@ standard `ci:gfx...` labels. | |
| > for that architecture. If no test runner exists for the family, the label is | ||
| > ignored with a warning. | ||
|
|
||
| #### Automated submodule update pull requests | ||
|
|
||
| The [submodule bump automation](../../build_tools/github_actions/bump_automation.py) | ||
| adds `ci:run-all-archs` to its pull requests to cover all GPU families. | ||
| `rocm-systems` and `rocm-libraries` updates also receive `ci:host-asan` for | ||
| host-ASan build coverage, plus `gfx950-dcgpu` and `gfx125X-dcgpu` to enable | ||
| label-gated tests on limited hardware. `rocgdb` updates receive `test:rocgdb`; | ||
| `mesa-fork` updates receive `test:rocdecode` and `test:rocjpeg` to restrict tests | ||
| to the affected projects. | ||
|
|
||
| Other pull requests can use the same sanitizer labels to opt in, or remove all | ||
| sanitizer labels to opt out. Submodule changes alone do not enable sanitizer CI. | ||
| For LLVM promotion pull requests, optionally add `ci:asan-debug` to exercise | ||
| device instrumentation and debug-info generation with the new compiler. | ||
|
|
||
| Sanitizer labels do not override GPU test gating. GPU sanity checks and | ||
| component tests remain subject to trigger and runner restrictions. CPU tests | ||
| may still run. `ci:skip` still skips CI. | ||
|
Comment on lines
+96
to
+98
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary/confusing detail here (often seen with "low effort level" AI written docs). Please trim. This whole paragraph could just be deleted.
|
||
|
|
||
| ### Push | ||
|
|
||
| CI runs on pushes to `main` if modified files pass the filters in | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just merged #9018 which validates that some known labels conflicts are not present. Let's make it so only one
ci:*asanlabel at a time can be added and if multiple are seen the CI fails. Then we won't need this priority list.