Cross Compile Pt 1: Add optional string to device_name functions#4771
Cross Compile Pt 1: Add optional string to device_name functions#4771
Conversation
There was a problem hiding this comment.
Pull request overview
This PR is the first step toward cross-compilation support on the GPU target by allowing callers to provide an explicit gfx/device string when querying device capabilities, instead of always querying the active HIP device.
Changes:
- Extend several
migraphx::gpu::device_namefeature-detection APIs to accept an optionalstd::string(defaulting to current-device behavior). - Add a shared helper in
device_name.cppto normalize the passed-in device/gfx string (trim and strip:suffix).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/targets/gpu/include/migraphx/gpu/device_name.hpp |
Updates exported device capability query APIs to accept an optional device/gfx string. |
src/targets/gpu/device_name.cpp |
Implements the new parameterized signatures and adds get_gfx_name() to normalize the input string. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| MIGRAPHX_GPU_EXPORT bool gfx_has_fp8fnuz_intrinsics(const std::string& device_name = ""); | ||
|
|
||
| MIGRAPHX_GPU_EXPORT bool gfx_has_fp8ocp_intrinsics(); | ||
| MIGRAPHX_GPU_EXPORT bool gfx_has_fp8ocp_intrinsics(const std::string& device_name = ""); | ||
|
|
||
| MIGRAPHX_GPU_EXPORT bool gfx_has_bf16_intrinsics(); | ||
| MIGRAPHX_GPU_EXPORT bool gfx_has_bf16_intrinsics(const std::string& device_name = ""); |
There was a problem hiding this comment.
These are MIGRAPHX_GPU_EXPORT APIs; changing the function signatures (adding a new parameter) changes the mangled symbol names and removes the old no-arg symbols. If any consumers link against a prebuilt migraphx GPU library, this is an ABI break. Consider keeping the original no-arg overloads (forwarding to the new string-taking overloads) and adding the new overloads separately.
src/targets/gpu/device_name.cpp
Outdated
| const auto& name = device_name.empty() ? get_device_name() : device_name; | ||
| return trim(split_string(name, ':').front()); |
There was a problem hiding this comment.
In get_gfx_name, device_name.empty() ? get_device_name() : device_name yields a temporary std::string, so this will copy device_name when it is non-empty. If you want to avoid that extra copy, use an if-statement to select between device_name (by reference) and the string returned from get_device_name(), or refactor to avoid the conditional operator here.
| const auto& name = device_name.empty() ? get_device_name() : device_name; | |
| return trim(split_string(name, ':').front()); | |
| if(device_name.empty()) | |
| return trim(split_string(get_device_name(), ':').front()); | |
| return trim(split_string(device_name, ':').front()); |
| MIGRAPHX_GPU_EXPORT bool gfx_has_fp8fnuz_intrinsics(const std::string& device_name = ""); | ||
|
|
||
| MIGRAPHX_GPU_EXPORT bool gfx_has_fp8ocp_intrinsics(); | ||
| MIGRAPHX_GPU_EXPORT bool gfx_has_fp8ocp_intrinsics(const std::string& device_name = ""); |
There was a problem hiding this comment.
The new optional device_name parameter is not exercised anywhere in the test suite (all current callers use the default). Since this change is meant to enable cross-compilation behavior, please add tests that call these helpers with explicit strings (e.g., "gfx90a", "gfx942", "gfx1200", and a value containing ':'), and assert the expected feature-detection results.
Motivation
For cross compilation, allowing the user to pass in a string for the gfx target is the first step.
Technical Details
This change updates our device_name function to accept a string in order to get various properties about the device.
Changelog Category
Add a
CHANGELOG.mdentry for any option other thanNot Applicable