Skip to content

Add definition of sycl-nvptx64-nvidia-cuda pass #186

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
merged 2 commits into from
Apr 9, 2025

Conversation

Pennycook
Copy link
Contributor

Related issues

Proposed changes

  • Add a regression test using -fsycl-targets=nvptx64-nvidia-cuda.
  • Define the missing sycl-nvptx64-nvidia-cuda pass.

"nvptx64-nvidia-cuda" should be a valid target.

Signed-off-by: John Pennycook <[email protected]>
@Pennycook Pennycook added this to the 2.0.0 milestone Apr 9, 2025
@Pennycook Pennycook added the bug Something isn't working label Apr 9, 2025
Copy link

@laserkelvin laserkelvin left a comment

Choose a reason for hiding this comment

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

LGTM - is there an exhaustive set of fsycl-targets documented somewhere? I did a quick search and this seems to be it

@Pennycook
Copy link
Contributor Author

LGTM - is there an exhaustive set of fsycl-targets documented somewhere? I did a quick search and this seems to be it

I don't think so. I think the idea is that other plugins or LLVM variants could add their own targets here.

Users should be able to work around that by defining custom compiler behavior (i.e., defining a new pass for icpx associated with the right mode). But because compiling SYCL for NVIDIA is so common, I think it makes sense for us to include the definition by default.

@Pennycook Pennycook merged commit 435636f into P3HPC:main Apr 9, 2025
4 checks passed
@Pennycook Pennycook deleted the bugfix/sycl-nvptx64 branch April 9, 2025 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants