-
Notifications
You must be signed in to change notification settings - Fork 780
CMake: When rocminfo is present, ask users to explicitly enable or disable ROCm testing.
#22478
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
Conversation
Signed-off-by: Benoit Jacob <[email protected]>
Signed-off-by: Benoit Jacob <[email protected]>
| # But silently continuing with ROCm testing disabled by default has also been | ||
| # surprising. | ||
| if ((IREE_HAL_DRIVER_HIP OR IREE_HAL_DRIVER_AMDGPU) AND NOT DEFINED CACHE{IREE_HIP_TEST_TARGET_CHIP}) | ||
| find_program(_ROCMINFO_COMMAND rocminfo) |
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.
You could try using offload-arch (formerly amdgpu-arch) instead, as that is more generic, available on Windows too, and produces more machine-friendly output
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 don't have offload-arch in my installation (rocm 7.0), and amdgpu-arch have llvm version suffixes, which would make it a bit tricky to select...
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.
Wait this is great but the only place I can see offload-arch is in my LLVM directory from a tarball. AFAICS, if I hadn't switched to a llvm tarball as my toolchain (e.g. if I were still using the system toolchain) I wouldn't have this program at all?
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.
There is a amdgpu-arch under /opt/rocm-7.0.x but not in PATH - it's in some LLVM toolchain subdir.
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.
Normally I'd be thrilled to switch to building offload-arch ourselves from the LLVM tree but here I need this information early in configure.
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.
Available on Windows too
Is rocminfo not available on Windows?
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.
Both have offload-arch / amdgpu-arch.
Windows has hipinfo.
Linux has rocminfo and rocm_agent_enumerator.
I'm still figuring out how to eventually turn that into something sane.
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.
No urgency on merging this .... this PR is just cherries on cakes.
One safe but kinda suboptimal resolution is to just build the offload-arch target from our llvm-project tree. Then give up on producing a configure-time diagnostic and instead produce a ctest-time diagnostic, which also removes the assumption that build host == test host.
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.
Looks fine to me but I'm not an expert and haven't tried running this. It would be good to get at least one more approval.
|
@kuhar and I decided to just merge this as-is because however imperfect, it's better than the current state. When Anyway, can't merge at the moment due to CI breakage. |
This is a leading cause of surprises and caveats for new users - it's something that is not naturally fitting in generic CMake instructions, but is important for people setting up for ROCm development.