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

Enable building to target AMD GPUs #1731

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ndgrigorian
Copy link
Collaborator

This PR modifies cmake scripts throughout dpctl to enable building for AMD. This is done by either setting the DPCTL_TARGET_AMD environment variable to the intended build architecture, or using -DDPCTL_TARGET_AMD.

_dpctl_sycl_target_compile_options and _dpctl_sycl_target_link_options cmake lists are used to prevent branching logic in later scripts.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • Have you checked performance impact of proposed changes?
  • If this PR is a work in progress, are you opening the PR as a draft?

Copy link

@coveralls
Copy link
Collaborator

coveralls commented Jul 13, 2024

Coverage Status

coverage: 87.883%. remained the same
when pulling c53fb26 on feature/enable-amd-builds
into 08b3517 on master.

Copy link

Array API standard conformance tests for dpctl=0.18.0dev0=py310h15de555_104 ran successfully.
Passed: 894
Failed: 15
Skipped: 105

Copy link

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_180 ran successfully.
Passed: 893
Failed: 2
Skipped: 119

@oleksandr-pavlyk oleksandr-pavlyk changed the title Enable building for AMD Enable building to target AMD GPUs Aug 7, 2024
Add DPCTL_TARGET_AMD cmake variable which is a string referring to the architecture to build for
Prevents build failure on AMD
Copy link

Array API standard conformance tests for dpctl=0.18.0dev0=py310ha798474_311 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

@oleksandr-pavlyk
Copy link
Collaborator

@ndgrigorian Regrettably use of sycl::log1p change necessary to enable compiling for AMD breaks compiling for CUDA.

Perhaps a preprocessor variable can be used to enable building for SPV/NVPTX or SPV/AMDGCN targets, but not for all three except after the bug gets fixed. It may be possible to write implementation of log1p to enable building for all three too.

@ndgrigorian
Copy link
Collaborator Author

@ndgrigorian Regrettably use of sycl::log1p change necessary to enable compiling for AMD breaks compiling for CUDA.

Perhaps a preprocessor variable can be used to enable building for SPV/NVPTX or SPV/AMDGCN targets, but not for all three except after the bug gets fixed. It may be possible to write implementation of log1p to enable building for all three too.

Yes, I only added the commit to make it convenient for the build failure to be reproduced.

Writing our own implementation is possible, too. I think that would be preferable, but on the other hand, it's a corner case to build for both CUDA and AMD.

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.

None yet

3 participants