Skip to content

[SYCL] Add missing supported AMDGPU architectures to SYCL #15723

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

Merged

Conversation

GeorgeWeb
Copy link
Contributor

@GeorgeWeb GeorgeWeb commented Oct 16, 2024

The three added gfx7 architectures were previously omitted from the list of supported ones for SYCL targeting AMDGPU.
There is no rational for them to be excluded rather than a potential mistake when all the rest were added, so this PR adds them in.
All of these exactly match the LLVM AMDGPU support docs here: https://llvm.org/docs/AMDGPUUsage.html#processors.
This also makes reusing the current OffloadArch enum in clang Driver easier with no concern of whether we have to filter these out for SYCL.

@GeorgeWeb GeorgeWeb force-pushed the georgi/update-sycl-supported-arch-docs branch from 5d7468d to 0d4bebb Compare October 16, 2024 14:42
@GeorgeWeb GeorgeWeb marked this pull request as ready for review October 16, 2024 15:00
@GeorgeWeb GeorgeWeb requested review from a team as code owners October 16, 2024 15:00
Copy link
Contributor

@keryell keryell left a comment

Choose a reason for hiding this comment

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

Thanks!

@GeorgeWeb
Copy link
Contributor Author

@intel/llvm-reviewers-runtime gentle ping. Not a prio, just good to get in to support one of the new driver changes wrt allowed --offload-arch targets. Thanks!

@npmiller
Copy link
Contributor

npmiller commented Nov 5, 2024

ping @intel/llvm-reviewers-runtime @sergey-semenov Could we get a review on this, we're still missing one approval, and it's a fairly small change so it should be quick to review.

Copy link
Contributor

@maarquitos14 maarquitos14 left a comment

Choose a reason for hiding this comment

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

LGTM.

@npmiller
Copy link
Contributor

npmiller commented Nov 5, 2024

@intel/llvm-gatekeepers this should be ready to merge

@sommerlukas sommerlukas merged commit 4452bc1 into intel:sycl Nov 5, 2024
14 checks passed
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.

8 participants