hipamd icon indicating copy to clipboard operation
hipamd copied to clipboard

cmake: use property list for GPU_TARGETS

Open psychocoderHPC opened this issue 4 years ago • 4 comments

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

  • Is keeps AMDGPU_TARGETS defines the variable deprecated and mark it as advanced to avoid confusing new users.
  • If the user is setting AMD_GPU_TARGETS in the initial cmake call e.g. cmake -DAMDGPU_TARGETS=gfx908;gfx906 a deprecation warning is thrown and the value of AMDGPU_TARGETS is assigned to GPU_TARGETS
  • GPU_TARGETS can be changed with ccmake by pressing enter, all supported architectures, without features, will be available.
  • 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+;gfx906

This PR is based on https://github.com/ROCm-Developer-Tools/HIP/pull/2289 which was closed due to the newly opened HIP develop branch. In the original PR @pfultz2 noted that the drop-down list for GPU_TARGETS is 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_TARGETS instead of using ccmake. 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.

psychocoderHPC avatar Aug 06 '21 08:08 psychocoderHPC

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

psychocoderHPC avatar Aug 06 '21 08:08 psychocoderHPC

@pfultz2 Is there anything to do before this can be merged?

psychocoderHPC avatar Sep 01 '21 15:09 psychocoderHPC

ping

psychocoderHPC avatar Nov 23 '21 15:11 psychocoderHPC

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

saadrahim avatar Aug 18 '22 21:08 saadrahim