cmake: use property list for GPU_TARGETS #1
Conversation
|
The list of supported target is taken from: https://github.com/ROCm-Developer-Tools/HIP/blob/main/docs/markdown/hip_porting_guide.md#compiler-options-supported-on-amd-platforms |
hip-config.cmake.in
Outdated
| set(AMDGPU_TARGETS "gfx900;gfx906;gfx908;gfx90a;gfx1030" CACHE STRING "AMD GPU targets to compile for") | ||
| set(GPU_TARGETS "${AMDGPU_TARGETS}" CACHE STRING "GPU targets to compile for") | ||
| set(GPU_DEFAULT_TARGETS "gfx900;gfx906;gfx908") | ||
| set(GPU_SUPPORTED_TARGETS "gfx701;gfx801;gfx802;gfx803;gfx900;gfx906;gfx908;gfx1010;gfx1011;gfx1012;gfx1030;gfx1031") |
There was a problem hiding this comment.
This is not the complete list of supported targets as this is missing the target features which is part of the target name as well. Furthermore, iterating over all the target features and default feature is a pretty long list. I think it would be better to not have the variable at all.
There was a problem hiding this comment.
I will remove GPU_SUPPORTED_TARGETS but it helps beginners to select an architecture even if the feature will not be directly chooseable.
There was a problem hiding this comment.
I removed GPU_SUPPORTED_TARGETS
hip-config.cmake.in
Outdated
| set(GPU_TARGETS "${AMDGPU_TARGETS}" CACHE STRING "GPU targets to compile for") | ||
| set(GPU_DEFAULT_TARGETS "gfx900;gfx906;gfx908") | ||
| set(GPU_SUPPORTED_TARGETS "gfx701;gfx801;gfx802;gfx803;gfx900;gfx906;gfx908;gfx1010;gfx1011;gfx1012;gfx1030;gfx1031") | ||
| set(AMDGPU_TARGETS "${GPU_DEFAULT_TARGETS}" CACHE STRING "[Deprecated: please use GPU_TARGETS] AMD GPU targets to compile for. Changing this variable after cmake was called once will have NO effect!") |
There was a problem hiding this comment.
I think it would be better to not define the variable, and then just check if its defined:
if(NOT DEFINED AMDGPU_TARGETS)
set(GPU_TARGETS "${GPU_DEFAULT_TARGETS}" CACHE STRING "GPU targets to compile for. Adding features is supported e.g. gfx908:xnack+")
else()
message(WARNING "Variable AMDGPU_TARGETS is deprecated! Please use GPU_TARGETS to choose architectures.")
set(GPU_TARGETS "${AMDGPU_TARGETS}" CACHE STRING "GPU targets to compile for. Adding features is supported e.g. gfx908:xnack+")
endif()There was a problem hiding this comment.
Thx for the suggestion. I took your suggestion and added you as co-author.
psychocoderHPC
force-pushed
the
topic-cmakeSetGPUArchitecture
branch
from
c1a0df8 to
1707c6c
Compare
This PR deprecates the variable `AMDGPU_TARGETS`. To choose architectures it is required to set `GPU_TARGETS`. Before this PR `GPU_TARGETS` was set to `AMDGPU_TARGETS`. Calling `cmake` again with a new value for `AMDGPU_TARGETS` is not propagated to `GPU_TARGETS` therefore the user is building against the initially selected architecture which is not obvious. This PR - If the user is setting `AMD_GPU_TARGETS` in the **initial cmake call** e.g. `cmake -DAMDGPU_TARGETS=gfx908` a deprecation warning is thrown and the value of `AMDGPU_TARGETS` is assigned to `GPU_TARGETS` - `GPU_TARGETS` can be any time overwritten by a user-defined value which allows an expert user to select architecture features too, e.g. `cmake -DGPU_TARGETS=gfx908:xnack+` Co-authored-by: Paul Fultz II <[email protected]>
psychocoderHPC
force-pushed
the
topic-cmakeSetGPUArchitecture
branch
from
1707c6c to
0e97622
Compare
|
@pfultz2 Is there anything to do before this can be merged? |
|
ping |
|
@gargrahul and @mangupta please take a look at this PR. There are two variable for the same functionality. GPU_TARGETS is the canonical variable. Let's take this advice and deprecate the AMDGPU_TARGETS variable. |
3 participants
This PR deprecates the variable
AMDGPU_TARGETS. To choose architectures it is required to setGPU_TARGETS.Before this PR
GPU_TARGETSwas set toAMDGPU_TARGETS. Callingcmakeagain with a new value forAMDGPU_TARGETSis not propagated toGPU_TARGETStherefore the user is building against the initially selected architecture which is not obvious.This PR
AMDGPU_TARGETSdefines the variable deprecated and mark it as advanced to avoid confusing new users.AMD_GPU_TARGETSin the initial cmake call e.g.cmake -DAMDGPU_TARGETS=gfx908;gfx906a deprecation warning is thrown and the value ofAMDGPU_TARGETSis assigned toGPU_TARGETSGPU_TARGETScan be changed withccmakeby pressing enter, all supported architectures, without features, will be available.GPU_TARGETScan be any time overwritten by a user-defined value which allows an expert user to select architecture features too, e.g.cmake -DGPU_TARGETS=gfx908:xnack+;gfx906This PR is based on ROCm/hip#2289 which was closed due to the newly opened
HIP developbranch.In the original PR @pfultz2 noted that the drop-down list for
GPU_TARGETSis not containing the architectures with features, due to the high number of combinations I have not added features to this PR.The feature can always be selected by an expert user if needed by setting
GPU_TARGETSinstead of usingccmake.Additionally, features are not in the current cmake, therefore this PR is not reducing any functionality.
If the features should be part of the drop-down list I suggest adding them in a follow-up PR by someone who knows which architectures can be combined with which feature.