From e0ac11f682ff4bb8f93eaf3f3113df55b1f29056 Mon Sep 17 00:00:00 2001 From: Hongtao Zhang Date: Wed, 25 Mar 2026 22:42:13 +0000 Subject: [PATCH 1/3] Support AMDGPU_TARGETS in ROCm CMake builds --- .../micro_benchmarks/rocm_common.cmake | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/superbench/benchmarks/micro_benchmarks/rocm_common.cmake b/superbench/benchmarks/micro_benchmarks/rocm_common.cmake index 1d2cc3934..0489e6cd0 100644 --- a/superbench/benchmarks/micro_benchmarks/rocm_common.cmake +++ b/superbench/benchmarks/micro_benchmarks/rocm_common.cmake @@ -37,11 +37,25 @@ else() set(HIP_PATH $ENV{HIP_PATH}) endif() -# Turn off CMAKE_HIP_ARCHITECTURES Feature if cmake version is 3.21+ -if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.21.0) - set(CMAKE_HIP_ARCHITECTURES OFF) +# Set HIP architectures from AMDGPU_TARGETS environment variable if available. +# AMDGPU_TARGETS should be a space-separated list of GPU architectures, +# e.g. "gfx908 gfx90a gfx942". +# Both the CMake variable AMDGPU_TARGETS and CMAKE_HIP_ARCHITECTURES must be set: +# - AMDGPU_TARGETS is read by ROCm's hip-config-amd.cmake to set --offload-arch flags +# - CMAKE_HIP_ARCHITECTURES is the native CMake variable for HIP (requires >= 3.21) +if(DEFINED ENV{AMDGPU_TARGETS}) + string(REPLACE " " ";" HIP_ARCH_LIST "$ENV{AMDGPU_TARGETS}") + set(AMDGPU_TARGETS ${HIP_ARCH_LIST} CACHE STRING "AMD GPU targets to compile for") + if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.21.0) + set(CMAKE_HIP_ARCHITECTURES ${HIP_ARCH_LIST}) + endif() + message(STATUS "Using AMDGPU_TARGETS from environment: ${HIP_ARCH_LIST}") +else() + if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.21.0) + set(CMAKE_HIP_ARCHITECTURES OFF) + endif() + message(STATUS "AMDGPU_TARGETS not set, relying on hipcc auto-detection") endif() -message(STATUS "CMAKE HIP ARCHITECTURES: ${CMAKE_HIP_ARCHITECTURES}") if(EXISTS ${HIP_PATH}) # Search for hip in common locations From 0652f381e8bc20cd3b49ba31745dbc9683e50959 Mon Sep 17 00:00:00 2001 From: Hongtao Zhang Date: Mon, 27 Apr 2026 21:49:48 +0000 Subject: [PATCH 2/3] Address PR review comments for AMDGPU_TARGETS handling - Clarify comment: micro-benchmarks are CXX-only via hipcc; CMAKE_HIP_ARCHITECTURES is set only for compatibility with HIP-language projects, not required here. - Normalize whitespace via STRIP + REGEX REPLACE [ \t]+ to avoid empty list elements from leading/trailing or repeated whitespace. - Add FORCE to the cache set so a changed AMDGPU_TARGETS env value overrides any stale cached value on reconfigure. - Replace DEFINED ENV check with non-empty HIP_ARCH_LIST check so an empty or whitespace-only AMDGPU_TARGETS falls back to hipcc auto-detection. --- .../micro_benchmarks/rocm_common.cmake | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/superbench/benchmarks/micro_benchmarks/rocm_common.cmake b/superbench/benchmarks/micro_benchmarks/rocm_common.cmake index 0489e6cd0..2a4d0a520 100644 --- a/superbench/benchmarks/micro_benchmarks/rocm_common.cmake +++ b/superbench/benchmarks/micro_benchmarks/rocm_common.cmake @@ -38,14 +38,22 @@ else() endif() # Set HIP architectures from AMDGPU_TARGETS environment variable if available. -# AMDGPU_TARGETS should be a space-separated list of GPU architectures, +# AMDGPU_TARGETS should be a whitespace-separated list of GPU architectures, # e.g. "gfx908 gfx90a gfx942". -# Both the CMake variable AMDGPU_TARGETS and CMAKE_HIP_ARCHITECTURES must be set: -# - AMDGPU_TARGETS is read by ROCm's hip-config-amd.cmake to set --offload-arch flags -# - CMAKE_HIP_ARCHITECTURES is the native CMake variable for HIP (requires >= 3.21) -if(DEFINED ENV{AMDGPU_TARGETS}) - string(REPLACE " " ";" HIP_ARCH_LIST "$ENV{AMDGPU_TARGETS}") - set(AMDGPU_TARGETS ${HIP_ARCH_LIST} CACHE STRING "AMD GPU targets to compile for") +# In this repository's micro-benchmarks, AMDGPU_TARGETS is what actually drives +# --offload-arch selection, via ROCm's hip-config-amd.cmake and hipcc (the C++ +# compiler). CMAKE_HIP_ARCHITECTURES is set (when supported by CMake >= 3.21) +# for compatibility with projects that enable the CMake HIP language, but is +# not required for these CXX-only projects. +set(_amdgpu_targets_raw "$ENV{AMDGPU_TARGETS}") +string(STRIP "${_amdgpu_targets_raw}" _amdgpu_targets_stripped) +# Collapse runs of spaces/tabs into a single ';' to avoid empty list elements +# from leading/trailing or repeated whitespace (e.g., "gfx90a gfx942"). +string(REGEX REPLACE "[ \t]+" ";" HIP_ARCH_LIST "${_amdgpu_targets_stripped}") +if(NOT HIP_ARCH_LIST STREQUAL "") + # Use FORCE so the environment variable wins over any stale cached value + # when the build directory is reconfigured with a different AMDGPU_TARGETS. + set(AMDGPU_TARGETS ${HIP_ARCH_LIST} CACHE STRING "AMD GPU targets to compile for" FORCE) if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.21.0) set(CMAKE_HIP_ARCHITECTURES ${HIP_ARCH_LIST}) endif() @@ -54,7 +62,7 @@ else() if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.21.0) set(CMAKE_HIP_ARCHITECTURES OFF) endif() - message(STATUS "AMDGPU_TARGETS not set, relying on hipcc auto-detection") + message(STATUS "AMDGPU_TARGETS not set (or empty), relying on hipcc auto-detection") endif() if(EXISTS ${HIP_PATH}) From 3800dbd5ac0287a9b554d6dccc7cbc6873656eb3 Mon Sep 17 00:00:00 2001 From: Hongtao Zhang Date: Sun, 3 May 2026 04:58:46 +0000 Subject: [PATCH 3/3] Address review feedback for AMDGPU_TARGETS handling - Broaden the separator regex to accept whitespace, ',', and ';' so values like 'gfx908,gfx90a' or CMake-list 'gfx908;gfx90a' are parsed correctly instead of becoming a single bogus token. - Drop CACHE STRING ... FORCE: use a normal directory-scoped variable for AMDGPU_TARGETS so we do not pollute the global CMake cache or override the variable in nested projects. The env var is re-read on every reconfigure, so this still wins over stale state. --- .../micro_benchmarks/rocm_common.cmake | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/superbench/benchmarks/micro_benchmarks/rocm_common.cmake b/superbench/benchmarks/micro_benchmarks/rocm_common.cmake index 2a4d0a520..aea79c7aa 100644 --- a/superbench/benchmarks/micro_benchmarks/rocm_common.cmake +++ b/superbench/benchmarks/micro_benchmarks/rocm_common.cmake @@ -37,9 +37,9 @@ else() set(HIP_PATH $ENV{HIP_PATH}) endif() -# Set HIP architectures from AMDGPU_TARGETS environment variable if available. -# AMDGPU_TARGETS should be a whitespace-separated list of GPU architectures, -# e.g. "gfx908 gfx90a gfx942". +# Set HIP architectures from the AMDGPU_TARGETS environment variable if available. +# Accepts the common separators a user might pass: whitespace, ',', or ';' +# (e.g. "gfx908 gfx90a gfx942", "gfx908,gfx90a", or "gfx908;gfx90a"). # In this repository's micro-benchmarks, AMDGPU_TARGETS is what actually drives # --offload-arch selection, via ROCm's hip-config-amd.cmake and hipcc (the C++ # compiler). CMAKE_HIP_ARCHITECTURES is set (when supported by CMake >= 3.21) @@ -47,13 +47,15 @@ endif() # not required for these CXX-only projects. set(_amdgpu_targets_raw "$ENV{AMDGPU_TARGETS}") string(STRIP "${_amdgpu_targets_raw}" _amdgpu_targets_stripped) -# Collapse runs of spaces/tabs into a single ';' to avoid empty list elements -# from leading/trailing or repeated whitespace (e.g., "gfx90a gfx942"). -string(REGEX REPLACE "[ \t]+" ";" HIP_ARCH_LIST "${_amdgpu_targets_stripped}") +# Collapse runs of any common separator (spaces, tabs, CR/LF, ',', ';') into a +# single ';' so the result is a well-formed CMake list with no empty elements. +string(REGEX REPLACE "[ \t\r\n,;]+" ";" HIP_ARCH_LIST "${_amdgpu_targets_stripped}") if(NOT HIP_ARCH_LIST STREQUAL "") - # Use FORCE so the environment variable wins over any stale cached value - # when the build directory is reconfigured with a different AMDGPU_TARGETS. - set(AMDGPU_TARGETS ${HIP_ARCH_LIST} CACHE STRING "AMD GPU targets to compile for" FORCE) + # Use a normal (non-cache) directory-scoped variable so we do not pollute + # the global CMake cache or override AMDGPU_TARGETS in nested projects. + # The env var is re-read on every reconfigure, so this still wins over + # stale state when the user changes AMDGPU_TARGETS. + set(AMDGPU_TARGETS ${HIP_ARCH_LIST}) if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.21.0) set(CMAKE_HIP_ARCHITECTURES ${HIP_ARCH_LIST}) endif()