Skip to content
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

changed the CMake option from AMDGPU_TARGETS to GPU_TARGETS #1435

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

akolliasAMD
Copy link
Collaborator

@akolliasAMD akolliasAMD commented Nov 26, 2024

Details

Changed cmake to accept GPU_TARGETS as the option for target different gpu targets

Work item: _"Internal"

Why were the changes made?
Rocm general change

Approval Checklist

Do not approve until these items are satisfied.

  • Verify the CHANGELOG has been updated, if
    • there are any NCCL API version changes,
    • any changes impact library users, and/or
    • any changes impact any other ROCm library.

@nileshnegi
Copy link
Collaborator

nileshnegi commented Nov 26, 2024

might need to change ./install.sh and README.md for the install.sh -h output

@akolliasAMD akolliasAMD requested a review from a team as a code owner November 26, 2024 20:50
if [[ ! -z "${build_amdgpu_targets}" ]]; then
cmake_common_options="${cmake_common_options} -DAMDGPU_TARGETS=${build_amdgpu_targets}"
if [[ ! -z "${build_gpu_targets}" ]]; then
cmake_common_options="${cmake_common_options} -DAMDGPU_TARGETS=${build_gpu_targets}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-DGPU_TARGETS?

@amd-jnovotny
Copy link
Contributor

amd-jnovotny commented Nov 26, 2024

@akolliasAMD: This is going to collide with #1427. I don't think all of your changes are applicable to 6.3, so we might want to get my changes in first so I can backport to 6.3.0 without picking up these changes. I'm just waiting for a manager approval from docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants