Skip to content

[SYCL] Implement device image properties for virtual functions #14875

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

Conversation

AlexeySachkov
Copy link
Contributor

Implementation design explaining those changes in a bigger picture can be found in #10540

Key things implemented here:

  • device code split to outline virtual functions into separate device images
  • emission of new properties for virtual functions
  • generation of calls-indirectly LLVM IR attribute for kernels that construct objects with virtual functions, but don't do calls
  • device image manipulations to cleanup or preserve virtual functions depending on a device image

Even though those pieces are technically independent from each other, it is hard to split them apart into separate PRs, because they all have to be either present or absent for existing E2E tests for virtual functions to work.

Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

looks good! just some questions and generic typo suggestions

Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

lgtm thanks! sorry, i have an eye for typos

@AlexeySachkov
Copy link
Contributor Author

AMD runner seems to be offline. Virtual function E2E tests are disabled on CUDA & HIP anyway and all builds completed, I do not expect any impact on AMD from this PR, so I will proceed with merge.

@AlexeySachkov AlexeySachkov merged commit 6127715 into intel:sycl Aug 5, 2024
15 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.

2 participants