Skip to content

ci(rocjitsu): Add missing dependencies on executables - #10960

Merged
amd-eochoalo merged 3 commits into
developfrom
users/amd-eochoalo/2026-08-31/add-missing-dependencies
Sep 3, 2026
Merged

amd-eochoalo merged 3 commits into
developfrom
users/amd-eochoalo/2026-08-31/add-missing-dependencies

Conversation

@amd-eochoalo

@amd-eochoalo amd-eochoalo commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Making rocjitsu tests more portable across different versions of amdclang.

Technical Details

When switching between different nightly builds, tests compiled with amdclang may not be recompiled unless there is a strict dependency.

From the CMake documentation:

DEPENDS
Specify files on which the command depends. Each argument is converted to a dependency as follows:

[...] if the target is an executable or library, a file-level dependency is created to cause the custom command to re-run whenever the target is recompiled.

Issue Tracking

N/A.

Test Plan

Test on CI.

Test Result

Tests pass on CI.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• JIRA ID (on separate line)
ROCM-25757
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID (on separate line)
AIRUNTIME-2352
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
• Closes #10
• Fixes octo-org/octo-repo#100
• Resolves: #123
• #123
• https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@amd-eochoalo
amd-eochoalo marked this pull request as ready for review August 31, 2026 14:05
@amd-eochoalo
amd-eochoalo requested review from a team and atgutier as code owners August 31, 2026 14:05

@atgutier atgutier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] emulation/rocjitsu/tests/CMakeLists.txt:1815 — Configure-time KFD permissions can
permanently remove hardware tests

Reasoning: The new AND RJ_HAS_KFD_ACCESS prevents rocm_agent_enumerator from running whenever the
machine performing CMake configuration cannot access /dev/kfd. That conflates target discovery with
permission to execute tests on the configure host.

rocm_agent_enumerator intentionally supports several discovery paths that do not require KFD access:
ROCM_TARGET_LST, a colocated target.lst, sysfs topology, and PCI enumeration. The first two are
explicitly intended for sandboxed and containerized build environments.

When enumeration is skipped, every HAS_*_GPU variable remains false, so the conditional
rj_add_test() calls around lines 1910, 1921, 1941, 2084, 2250, 2442, and 2923 are never executed.
Because rj_add_test() also creates installed CTest entries only when called (tests/
CMakeLists.txt:216-308), a package configured without KFD access permanently omits those tests—even
if it is later installed and tested on a GPU worker with full access.

Evidence:

  • projects/rocminfo/rocm_agent_enumerator:127-144,263-303 performs non-KFD discovery before falling
    back to rocminfo.

  • projects/rocminfo/README.md:34-44 documents ROCM_TARGET_LST and target.lst specifically for
    sandbox/container environments.

  • The new condition at tests/CMakeLists.txt:1815 bypasses all those paths.

  • Many affected registrations supply INSTALL_COMMAND, so this changes the installed test manifest as
    well as the current build tree.

Suggested fix: Keep architecture discovery independent of /dev/kfd accessibility. Register the
applicable hardware tests based on discovered or explicitly supplied targets, then check KFD access
at test execution time and return a CTest skip result when it is unavailable. If build-tree and
installed-test behavior must differ, separate their registration paths.

Confidence: high. The remaining environmental question is which current CI/package flows configure
and execute on different hosts, but the supported installed-test and ROCM_TARGET_LST workflows are
unambiguously affected.

[MINOR] emulation/rocjitsu/tests/CMakeLists.txt:1065 — The access check misses the earlier gfx1201
hardware-test gate

Reasoning: RJ_HAS_GFX1201_GPU is still derived solely from readable sysfs topology and does not
incorporate the newly computed RJ_HAS_KFD_ACCESS. On a gfx1201 system where topology is visible
but /dev/kfd is not accessible, the six DbtGuestRocminfoTest.* cases under line 1680 remain
registered.

Those tests use guest_gfx950_on_gfx1201.json, which enables hardware-backed DBT guest mode. GuestKfd
then opens the real device with openat(..., "/dev/kfd", O_RDWR | O_CLOEXEC, ...) at
guest_kfd.cpp:587; an access failure propagates through initialization. rocminfo_test.cpp:156-159
treats the resulting nonzero process exit as a test failure rather than a skip.

Suggested fix: Include these earlier gfx1201 registrations in the runtime-access gating described
above. As a smaller immediate fix, require both RJ_HAS_GFX1201_GPU and RJ_HAS_KFD_ACCESS at line
1680.

Confidence: high. The trigger is the common configuration where KFD topology is world-readable but
the device node is unavailable to the current user.

@RattataKing
RattataKing requested a review from a team September 1, 2026 19:28

@RattataKing RattataKing left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding missing deps of ${AMDCXX} and ${CLANG_OFFLOAD_BUNDLER} looks correct to me

@amd-eochoalo

Copy link
Copy Markdown
Contributor Author

@atgutier I removed the code about kfd access, just adding missing dependencies.

@amd-eochoalo amd-eochoalo changed the title ci(rocjitsu): Add missing dependencies and skip tests if there is no KFD access ci(rocjitsu): Add missing dependencies on executables Sep 2, 2026
@amd-eochoalo
amd-eochoalo merged commit 1a163e4 into develop Sep 3, 2026
25 of 27 checks passed
@amd-eochoalo
amd-eochoalo deleted the users/amd-eochoalo/2026-08-31/add-missing-dependencies branch September 3, 2026 17:29
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants