[cjatin]

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 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.

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
#2373 (comment)
#2001 (comment)
#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

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

[ScottTodd]

Please link to the tracking issue #1354 in the PR description

Structurally this looks okay. We can't merge with the rocm-systems submodule changes here though (you may want to mark the PR as a draft until it is actually fit to be merged).

[ScottTodd]

Copying a comment from #2001

We should have the same artifact file for Linux and Windows. Let's change hip-tests so that is possible.

[cjatin]

I think this is being worked on.

[ScottTodd]

Okay, we can follow-up with that later. Baseline test coverage on both operating systems takes priority over getting the ideal setup consistently across platforms.

[jayhawk-commits]

rocm-systems submodule update that was merged should include the changes on that end that is needed for this PR

[jayhawk-commits]

This feedback got lost in the split into a new PR: #2001 (review)

To be clear, I'm not a fan of introducing the underscore in the file name as new separator for indicating the OS. If we're splitting into files for OS in the interim, have both TOML files indicate the OS with hyphen separator. When they can be combined into a single file, then we can go ahead without the OS indicator in the filename.

As requested earlier in the PR, please provide an update in the issue linked so that the progress over multiple PRs can be tracked, and link that issue in the PR description

[agunashe]

This feedback got lost in the split into a new PR: #2001 (review)

To be clear, I'm not a fan of introducing the underscore in the file name as new separator for indicating the OS. If we're splitting into files for OS in the interim, have both TOML files indicate the OS with hyphen separator. When they can be combined into a single file, then we can go ahead without the OS indicator in the filename.

As requested earlier in the PR, please provide an update in the issue linked so that the progress over multiple PRs can be tracked, and link that issue in the PR description

ROCm/rocm-systems#2187 - installation paths will be fixed in this and paths will become consistent for linux and windows.
When 2187 reaches Rock, we can remove the artifact-core-hiptests_win.toml file all together. Since this is temporary, it will help if we can check this in without waiting on other patches.

[jharryma]

Considering the artifacts will be unified after ROCm/rocm-systems#2187, this LGTM.

[ScottTodd]

Okay, we can follow-up with that later. Baseline test coverage on both operating systems takes priority over getting the ideal setup consistently across platforms.