Enable hip-tests on TheRock by cjatin · Pull Request #2001 · ROCm/TheRock

cjatin · 2025-11-03T17:11:06Z

Motivation

Enable hip-tests execution on TheRock linux and windows.
Aligns installation path of hip-tests on linux and Windows
enables sharding of hip-tests

Details

Enable hip-tests execution on TheRock linux and windows.

This PR fixes -
#1354

Hip-tests source that gets built/executed as part of this PR
https://github.com/ROCm/rocm-systems/tree/develop/projects/hip-tests/catch

Related PRs to enable build/execution of tests -
#2373 - enables building of tests
ROCm/rocm-systems#1624 - prepares rocm-systems to be built with catchlib from upstream , compiler/hip detection, packaging change
ROCm/rocm-systems#2187 - aligns installation path for linux and windows

few other script changes to have compatible builds on older build frameworks.
https://github.com/AMD-ROCm-Internal/build-infra/pull/316
https://github.com/AMD-Radeon-Driver/compute-utils/pull/169
https://github.com/AMD-Radeon-Driver/compute-utils/pull/171
hip-tests executed and verified successfully

Submission Checklist

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

jayhawk-commits · 2025-11-04T20:39:50Z

Do you have any workflow runs that combine the changes from this PR with the code changes from ROCm/rocm-systems#1624 ?

ScottTodd · 2025-11-05T21:23:20Z

+      # Non-Windows CLR options.
+      list(APPEND HIP_TESTS_RUNTIME_DEPS
+        hip-clr
+      )


Why is this only a runtime dep on Linux? hip-clr is available and used on Windows too

jayhawk-commits · 2025-11-07T16:05:14Z

Going to change the submodule of rocm-systems to match branch from ROCm/rocm-systems#1624

jayhawk-commits · 2025-11-07T16:08:06Z

Nevermind. Complaining about conflicts in the submodule, but that should be the sequence to test the changes.

jayhawk-commits · 2025-11-12T00:40:21Z

I don't see hip-tests build logs, but I do see the "including subproject" portion in the logs.

cjatin · 2025-11-13T11:57:17Z

I don't see hip-tests build logs, but I do see the "including subproject" portion in the logs.

Which CI job will have that information?

ScottTodd · 2025-11-13T16:39:22Z

I don't see hip-tests build logs, but I do see the "including subproject" portion in the logs.

Which CI job will have that information?

Get the build working locally first. I also see the configure completes: https://github.com/ROCm/TheRock/actions/runs/19278473408/job/55124041856#step:10:485 but the new subproject is not built (no longs in either the full build output or https://therock-ci-artifacts.s3.amazonaws.com/19278473408-linux/logs/gfx1151/index.html).

I suspect since there are no other subprojects depending on this project and the project does not provide an artifact, building --target therock-dist therock-archives does not build it. You should add an artifact, following https://github.com/ROCm/TheRock/blob/main/docs/development/artifacts.md

agunashe · 2025-11-13T20:00:10Z

I don't see hip-tests build logs, but I do see the "including subproject" portion in the logs.

Which CI job will have that information?

Get the build working locally first. I also see the configure completes: https://github.com/ROCm/TheRock/actions/runs/19278473408/job/55124041856#step:10:485 but the new subproject is not built (no longs in either the full build output or https://therock-ci-artifacts.s3.amazonaws.com/19278473408-linux/logs/gfx1151/index.html).

I suspect since there are no other subprojects depending on this project and the project does not provide an artifact, building --target therock-dist therock-archives does not build it. You should add an artifact, following https://github.com/ROCm/TheRock/blob/main/docs/development/artifacts.md

Added the artifacts for hip-tests. Hopefully the tests are now built

https://github.com/ROCm/TheRock/pull/2001/files#diff-8f52846645b80806e11fbe318820659ca6d81d2fe305d815bb97f62d2102e1fdR236

marbre · 2025-11-13T22:42:12Z

+      TARGETS USE_DIST_AMDGPU_TARGETS
+      DESCRIPTOR artifact-core-hiptests.toml
+      COMPONENTS
+        run


See above, should most likely be test.

sure thanks. ill make this change.

marbre · 2025-11-13T22:44:24Z

 redshift_connector==2.1.8
+
+# hip-tests
+rpm


What is rpm needed for?

hip-tests artifacts is packaged into deb, tar, rpm files.
So we need "sudo apt install rpm " on the system to generate the rpm artifact file.

I see linux packages are manually installed in the instructions. Any way to have this rpm package installed ?
or I can skip rpm for now

jayhawk-commits · 2025-11-14T03:20:28Z

I see hip-tests being built now.
gfx942 build logs: https://therock-ci-artifacts.s3.amazonaws.com/19349642564-linux/logs/gfx94X-dcgpu/index.html
gfx942 artifacts: https://therock-ci-artifacts.s3.amazonaws.com/19349642564-linux/index-gfx94X-dcgpu.html

jayhawk-commits · 2025-11-14T03:23:58Z

The sequence should now be:

get make hip-tests compileable with TheRock rocm-systems#1624 approved and merged
change back the submodule in this PR
get rocm-systems submodule bumped in TheRock
merge this PR

agunashe · 2025-11-14T03:26:58Z

The sequence should now be:

get make hip-tests compileable with TheRock rocm-systems#1624 approved and merged

change back the submodule in this PR

get rocm-systems submodule bumped in TheRock

merge this PR

we need to verify the test execution of the test binaries.

jayhawk-commits · 2025-11-14T03:29:37Z

The sequence should now be:

get make hip-tests compileable with TheRock rocm-systems#1624 approved and merged

change back the submodule in this PR

get rocm-systems submodule bumped in TheRock

merge this PR

we need to verify the test execution of the test binaries.

If you want to do that within this PR, then take a look at this example #1380
Add a section for hip-clr in fetch_test_configurations.py and create new test_*.py files.

mgehre-amd · 2025-11-25T22:29:15Z

I'm curious, why is the sudo needed to run ctest? I don't think the other tests have that.

agunashe · 2025-11-25T23:28:29Z

I'm curious, why is the sudo needed to run ctest? I don't think the other tests have that.

our tests generates some _test.cmake files which caches the test names at runtime. These files are part of catch/ctest framework. <install_path>/share/hip is a read only path. So the sudo was required to ovewrite this issue. Without it the _test.cmake files cannot be created by the catch/ctest framework causing the tests to fail

stellaraccident · 2025-11-26T05:45:18Z

I think I might have just mid-air collided with you in: 91dc209

Now artifacts have to be declared in BUILD_TOPOLOGY.toml in the root. You should get a CMake error telling you this once you rebase. If you run into problems with this, I can push an update to the branch tomorrow with the necessary changes but should be pretty mechanical (and it will fail on configure if done wrong so should be low cost to fix).

agunashe · 2025-11-27T01:32:33Z

I think I might have just mid-air collided with you in: 91dc209

Now artifacts have to be declared in BUILD_TOPOLOGY.toml in the root. You should get a CMake error telling you this once you rebase. If you run into problems with this, I can push an update to the branch tomorrow with the necessary changes but should be pretty mechanical (and it will fail on configure if done wrong so should be low cost to fix).

Thank you for the comment. I added the hip-tests artifact to the BUILD_TOPOLOGY.toml in the root.
the tests seems to be executed fine in the latest run

geomin12

looking good, just some cleanup comments

geomin12 · 2025-12-16T16:47:44Z

+    "hip-tests": {
+        "job_name": "hip-tests",
+        "fetch_artifact_args": "--tests",
+        "timeout_minutes": 120, # it failed for 120 mins, fix it when we address sharding.


can remove comment, also we are allotting 300 minutes total for tests.

right now, this runs for 480 minutes, which is pretty long!!! let's do 60 mins with 4 shards (we can adjust based on test results in the CI)

I have removed the comment and kept the timeout as 120.
after few days of execution, we can trim it down to a smaller number

## Motivation This change enables hip-tests to be built on TheRock. This is the first step in getting hip-tests building/running with TheRock. ## Technical Details Enable hip-tests to be built with TheRock, when user enables testing via flag `THEROCK_BUILD_TESTING`. [hip-tests](https://github.com/ROCm/rocm-systems/tree/develop/projects/hip-tests/catch) are collection of tests that are used to test the hip-runtime. A corresponding PR is also required in hip-tests to make it compatible with TheRock, which are being done [here](ROCm/rocm-systems#1624). Subsequent PRs are required to run the built hip-tests which will follow after this PR. ROCm/rocm-systems#2187 - PR will fix the paths to be consistent across linux and windows. Below issues will be fixed with it https://github.com/ROCm/TheRock/pull/2373/files#r2582804642 ROCm#2373 (comment) ROCm#2001 (comment) ROCm#2001 (comment) ## Test Plan The build should complete end to end without failures, next set of changes will enable the running of tests. ## Test Result NA ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

stellaraccident · 2026-01-08T18:29:34Z

Hi @cjatin - I noticed there's also PR #2706 ("Start running hip-tests on TheRock") which appears to cover similar ground to this PR.

Since this PR was updated more recently, I'm assuming this is the active effort. Could you confirm and consider converting #2706 to draft or closing it if it's superseded? That would help reduce confusion for reviewers.

Thanks for working on getting hip-tests running in CI - this is valuable testing infrastructure!

cjatin · 2026-01-09T00:17:57Z

Hi @cjatin - I noticed there's also PR #2706 ("Start running hip-tests on TheRock") which appears to cover similar ground to this PR.

Since this PR was updated more recently, I'm assuming this is the active effort. Could you confirm and consider converting #2706 to draft or closing it if it's superseded? That would help reduce confusion for reviewers.

Thanks for working on getting hip-tests running in CI - this is valuable testing infrastructure!

Hey @stellaraccident closed the other one.

jayhawk-commits

LGTM but this PR has "changes requested" status

Multi-sharding hip-tests can be done in a separate PR.

ScottTodd

Marking as "reviewed", and I "resolved" most of the open conversation threads. In case it gets lost in the noise, this thread is still open and has legitimate concerns about sharding and test suite size for running smoketests: #2001 (comment)

geomin12

looks great overall, I added a comment regarding sharding, which will help get this test to a reasonable test time

Another point, I notice that hip-test ci:smoke is lodged into the code, is it possible to enable smoke tests for this test as well? Any filters available for smoke tests?

geomin12 · 2026-01-12T20:07:34Z

+        "-I",
+        f"{index_start},{index_end}",


Suggested change

"-I",

f"{index_start},{index_end}",

# shards the tests by running a specific set of tests based on starting test (shard_index) and stride (total_shards)

"--tests-information",

f"{shard_index},,{total_shards}",

This will shard the tests properly with TheRock + ctest (this implementation only seems to pick up a specific set of tests)

That way, this will get a reasonable test time

This check with SHARD_INDEX kind of worked, because of how TheRock scheduled each shard. -j will cause issues.

geomin12 · 2026-01-12T20:12:16Z

+    cmd = [
+        "ctest",
+        "-I",
+        f"{index_start},{index_end}",


can we also include -j flag (parallelization)? if this is enabled and we get test times down to ~ 20 mins total, we won't need those test filters

We can not use -j right now with hip-tests. almost all HIP tests run the test on default device which will be device 0. When we run multi threaded tests, multiple tests will enqueue stuff on device 0 which can causes some tests to fail which run timing based checks.
Ideally this should be sorted, but it will take some time.

ScottTodd

LGTM when Geo approves. We can tune the set of tests that run at each stage either in this PR or (fast) follow-ups. This test coverage is important but we need to not let that detail slip.

cjatin · 2026-01-13T11:31:43Z

looks great overall, I added a comment regarding sharding, which will help get this test to a reasonable test time

Another point, I notice that hip-test ci:smoke is lodged into the code, is it possible to enable smoke tests for this test as well? Any filters available for smoke tests?

Replied on -j query.

Regarding smoke tests, that entry is in rocm-systems repo. hip-tests at the moment does not have a set of smoke tests, right now. There are some efforts in that direction, but it might take some time to gather a list of tests which can be categorized as smoke tests.

geomin12

As discussed with Joseph offline, we should land this PR as hip-tests is quite important to the ROCm org and to get these tests enabled. The 43 mins is not ideal, but I will work on sharding in a follow-up PR

Works here: https://github.com/ROCm/TheRock/actions/runs/20786256084/job/59901495774 (I had to rebase due to main branch mess)

## Motivation Enable hip-tests execution on TheRock linux and windows. Aligns installation path of hip-tests on linux and Windows enables sharding of hip-tests ## Details Enable hip-tests execution on TheRock linux and windows. **This PR fixes -** #1354 **Hip-tests source that gets built/executed as part of this PR** https://github.com/ROCm/rocm-systems/tree/develop/projects/hip-tests/catch **Related PRs to enable build/execution of tests -** #2373 - enables building of tests ROCm/rocm-systems#1624 - prepares rocm-systems to be built with catchlib from upstream , compiler/hip detection, packaging change ROCm/rocm-systems#2187 - aligns installation path for linux and windows **few other script changes to have compatible builds on older build frameworks.** https://github.com/AMD-ROCm-Internal/build-infra/pull/316 https://github.com/AMD-Radeon-Driver/compute-utils/pull/169 https://github.com/AMD-Radeon-Driver/compute-utils/pull/171 hip-tests executed and verified successfully ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Geo Min <geomin12@amd.com>

ScottTodd · 2026-01-22T17:45:05Z

Any updates or ETA on getting the "smoke" filter configured? We have data now showing that these newly added tests are contributing significantly to long queue times for our Windows gfx1151 test runners: #3050

github-project-automation Bot added this to TheRock Triage Nov 3, 2025

github-project-automation Bot moved this to TODO in TheRock Triage Nov 3, 2025

jayhawk-commits requested review from ScottTodd, jayhawk-commits and marbre November 4, 2025 20:34

jayhawk-commits reviewed Nov 4, 2025

View reviewed changes

Comment thread core/CMakeLists.txt Resolved

jayhawk-commits reviewed Nov 4, 2025

View reviewed changes

Comment thread core/CMakeLists.txt Resolved

jayhawk-commits mentioned this pull request Nov 4, 2025

make hip-tests compileable with TheRock ROCm/rocm-systems#1624

Merged

1 task

ScottTodd reviewed Nov 5, 2025

View reviewed changes

agunashe marked this pull request as ready for review November 11, 2025 20:32

marbre reviewed Nov 13, 2025

View reviewed changes

agunashe requested review from ScottTodd, agunashe and marbre and removed request for ScottTodd November 27, 2025 03:58

geomin12 reviewed Dec 16, 2025

View reviewed changes

HereThereBeDragons requested changes Dec 17, 2025

View reviewed changes

Enable running hip-tests on TheRock

5322e1e

cjatin force-pushed the users/jatin/hip-tests branch from da80bf4 to 5322e1e Compare January 7, 2026 11:49

Fix the ROCM_PATH lookup

d0525a7

stellaraccident mentioned this pull request Jan 8, 2026

Start running hip-tests on TheRock #2706

Closed

1 task done

jayhawk-commits requested review from HereThereBeDragons and ScottTodd January 9, 2026 16:05

jayhawk-commits approved these changes Jan 9, 2026

View reviewed changes

ScottTodd reviewed Jan 10, 2026

View reviewed changes

geomin12 self-requested a review January 12, 2026 17:26

geomin12 reviewed Jan 12, 2026

View reviewed changes

ScottTodd approved these changes Jan 12, 2026

View reviewed changes

subodh-dubey-amd force-pushed the main branch from 7e9ba03 to 3077ce8 Compare January 13, 2026 12:08

Merge branch 'main' into users/jatin/hip-tests

7836f05

geomin12 approved these changes Jan 13, 2026

View reviewed changes

HereThereBeDragons approved these changes Jan 13, 2026

View reviewed changes

cjatin merged commit f5524a6 into main Jan 13, 2026
17 checks passed

cjatin deleted the users/jatin/hip-tests branch January 13, 2026 20:17

github-project-automation Bot moved this from TODO to Done in TheRock Triage Jan 13, 2026

ScottTodd mentioned this pull request Jan 22, 2026

[CI] Long queue times for Windows gfx1151 runners (9 hours+) #3050

Closed

1 of 6 tasks

ScottTodd mentioned this pull request Feb 16, 2026

Build and run tests/samples from hip-tests #1354

Closed

ScottTodd mentioned this pull request Mar 26, 2026

Enable ocltst execution on Rock #3590

Draft

1 task done

-        "-I",
-        f"{index_start},{index_end}",
+       # shards the tests by running a specific set of tests based on starting test (shard_index) and stride (total_shards)
+    "--tests-information",
+    f"{shard_index},,{total_shards}",

Conversation

cjatin commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Details

Submission Checklist

Uh oh!

jayhawk-commits commented Nov 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jayhawk-commits commented Nov 7, 2025

Uh oh!

jayhawk-commits commented Nov 7, 2025

Uh oh!

jayhawk-commits commented Nov 12, 2025

Uh oh!

cjatin commented Nov 13, 2025

Uh oh!

ScottTodd commented Nov 13, 2025

Uh oh!

agunashe commented Nov 13, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jayhawk-commits commented Nov 14, 2025

Uh oh!

jayhawk-commits commented Nov 14, 2025

Uh oh!

agunashe commented Nov 14, 2025

Uh oh!

jayhawk-commits commented Nov 14, 2025

Uh oh!

mgehre-amd commented Nov 25, 2025

Uh oh!

agunashe commented Nov 25, 2025

Uh oh!

stellaraccident commented Nov 26, 2025

Uh oh!

agunashe commented Nov 27, 2025

Uh oh!

geomin12 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stellaraccident commented Jan 8, 2026

Uh oh!

cjatin commented Jan 9, 2026

Uh oh!

jayhawk-commits left a comment

cjatin commented Nov 3, 2025 •

edited

Loading

geomin12 left a comment •

edited

Loading