[andmar-amd]

Add kfdtest to TheRock CI

- Add kfdtest subproject with correct dependencies (statically links libhsakmt and LLVM)
- Use run_kfdtest.sh script which handles platform-specific test exclusions
- Add --kfdtest flag to install_rocm_from_artifacts.py
- Add kfdtest to BUILD_TOPOLOGY.toml (artifact group, build stage, and artifact)
- Add sysdeps to kfdtest artifact dependencies for bundled libraries
- Use get_target_property to dynamically locate ROCR-Runtime binary directory
- Set RPATH to lib/rocm_sysdeps/lib only (kfdtest statically links main deps)
- Support both standard and quick test modes via TEST_TYPE environment variable

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

[andmar-amd]

@copilot resolve the merge conflicts in this pull request

[Copilot]

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in commit 951302eb.

The two conflicts were:

• BUILD_TOPOLOGY.toml: Kept kfdtest in the compiler-runtime stage and removed rocrtst (which was moved to the runtime-tests stage in main).
• core/CMakeLists.txt: Aligned the new kfdtest block with the updated rocrtst structure from main, using if(THEROCK_BUILD_TESTING AND THEROCK_ENABLE_CORE_KFDTESTS) instead of the nested if pattern.

[saadrahim]

We should not hardcode this. Flaky SDMA queue tests are symptom of other underlying issues that need to be resolved.

[andmar-amd]

Greetings @saadrahim

Thank for the comment. This was just an attempt to see it would succeed here. Is there a way to get a node exclusively for kfdtest? Is does not play well with sharing the GPU, it has to have the node for itself . As for how long it would take to run this full test, it averages to about 20 minutes.

[saadrahim]

I believe you should consider it a requirement to make the tests work on a shared GPU. What feature does the test exercise that all GPUs are impacted? Can you separate the ones that cannot share GPUs because they test a feature that impacts all GPUs.

[andmar-amd]

@kentrussell could articulate better on this.

But it is not that it need all of the GPU nodes, it just needs 1 in the set, and it need to have all by itself or it may crash. FTR it does run on all the GPU node available, if it is not controlled with a '-n', and this usually will be green if there is no other process sharing that specific node. Since it looks like you have 8+ nodes on this ASIC , just show me how I can lock 1 exclusively for kfdtest and then I would release it when the test is complete.

[kentrussell]

While kfdtest can benefit from running on all GPUs, there is a lot of redundancy. The majority of subtests are testing the thunk+kernel interface, and it's not designed to be a HW validation tool like agfhc. Yes certain GPUs could throw failures in certain cases where previous ones did not, but it's rare. For TheRock CI, a single GPU is sufficient, since that covers the interfaces well enough, and speeds up the test by 7x
The big issue is that a number of subtests don't play well with others. Allocating all VRAM (or more), triggering faults, triggering GPU resets, etc. These all conflict with running other tests simultaneously

[saadrahim]

kfdtest is designed to test the driver. TheRock CI does not gate the driver releases. Therefore, updating the driver may break TheRock builds if a failure is introduced in the driver and is caught in TheRock CI through kfdtests. From a testability perspective, I do not support its enablement in TheRock. I think you need to consult with leadership on the CI side. Fixing a defect in the driver has no associated source code in TheRock as well.

@amd-aakash @suchitravemuri1 @ScottTodd @marbre

We also cannot publish kfdtests as a component versioned with ROCm, it needs to be versioned with the AMD GPU driver.

[saadrahim]

PLATFORM_MAP is defined but never referenced (the comment says detection is automatic) — dead code.
The large commented-out ADDITIONAL_EXCLUDE block is dead code; if container exclusions aren't needed in TheRock CI, delete it rather than ship it commented.
SHARD_INDEX/TOTAL_SHARDS defaults are ints (os.getenv(..., 1)) while env values are strings; it works because of later int(...) casts, but it's inconsistent.
In core/CMakeLists.txt, the existing rocrtst block gets re-indented (whitespace-only churn) just to nest the new if(... CORE_KFDTESTS) beside it. That inflates the diff and makes blame noisier — better to add the new block without reformatting the old one.

[geomin12]

works! https://github.com/ROCm/TheRock/actions/runs/27444211651/job/81144051501?pr=5377

but have some comments

[geomin12]

please provide better PR description

[nunnikri]

kfdtest is an internal amdgpu driver test. Its not testing user space ROCm. It need not be part of TheRock/ROCm ci. Or I would suggest to make it as 2 part change. bringing the kfdtest artifact to ROCm as first step and adding to TheRock ci be a different PR which need more consideration.
@saadrahim @amd-aakash could you clarify?

[saadrahim]

kfdtest is an internal amdgpu driver test. Its not testing user space ROCm. It need not be part of TheRock/ROCm ci. Or I would suggest to make it as 2 part change. bringing the kfdtest artifact to ROCm as first step and adding to TheRock ci be a different PR which need more consideration. @saadrahim @amd-aakash could you clarify?

Yes, adding it as a build artifact is the best approach. There is no need to add to CI for testing. kfdtests is only being built for historic purposes and should be removed in the future.

[nunnikri]

kfdtest is an internal amdgpu driver test. Its not testing user space ROCm. It need not be part of TheRock/ROCm ci. Or I would suggest to make it as 2 part change. bringing the kfdtest artifact to ROCm as first step and adding to TheRock ci be a different PR which need more consideration. @saadrahim @amd-aakash could you clarify?

Yes, adding it as a build artifact is the best approach. There is no need to add to CI for testing. kfdtests is only being built for historic purposes and should be removed in the future.

@andmar-amd do you want to remove build_tools/github_actions/test_executable_scripts/test_kfdtest.py based on this?

[therock-pr-bot]

❌ PR Check — Action Required

Check	Status	Details
🌿 Branch Name	✅ Pass	—
📝 PR Title/Description	❌ Fail	Error: Title does not follow Conventional Commits style. Expected: start with a valid type (feat, fix, docs, …). Desired format: type(optional-scope): short description ─── Error: PR description must reference a JIRA ID or ISSUE ID. Expected: include a JIRA ID or ISSUE ID line. The separator may be : or - (or omitted), and the value can be a JIRA key, a number (with or without #), or a link. Accepted examples: • JIRA ID : TESTAUTO-6039 • JIRA ID - #330 • JIRA ID #330 • ISSUE ID : TESTUTO-3334 • ISSUE ID #3334 • ISSUE ID - TESTAUTO-3433 • ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234 Current: no valid JIRA/ISSUE reference found
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	❌ Fail	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*). Current: code file(s) changed: build_tools/install_rocm_from_artifacts.py; no test file found
🔎 pre-commit	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

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

🚫 Please fix the failed policies

• ❌ PR Title/Description
• ❌ Unit Test

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.

[therock-pr-bot]

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

• ❌ PR Title/Description
• ❌ Unit Test

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

[andmar-amd]

Greetings @nunnikri @saadrahim

Yes I have completely remove the build_tools/github_actions/test_executable_scripts/test_kfdtest.py and other references to testing. As you can see AI review is now failing for missing unit test.

[saadrahim]

This is needed temporarily until we find a new home for kfdtests.

[nunnikri]

Discussed with @saadrahim. BU is good to release kfdtest as it is. So approving.

[jayhawk-commits]

Latest ASAN builds are failing on main due to kfdtest, I will revert this commit.
Example log: https://github.com/ROCm/TheRock/actions/runs/28172850345/job/83441457289

[stellaraccident]

Latest ASAN builds are failing on main due to kfdtest, I will revert this commit. Example log: https://github.com/ROCm/TheRock/actions/runs/28172850345/job/83441457289

[kfdtest] ld.lld: error: unable to find library -lhsakmt
[kfdtest] ld.lld: error: unable to find library -lnuma
[kfdtest] clang++: error: linker command failed with exit code 1 (use -v to see invocation)

This doesn't look like it would be as an specific and I would expect that it would impact the non asan builds too. It will need cmake plumbing changes to detect the proper in tree dependencies for those two deps.