-
Notifications
You must be signed in to change notification settings - Fork 769
[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
[SYCL] Implement device image properties for virtual functions #14875
Conversation
There was a problem hiding this 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
llvm/test/tools/sycl-post-link/device-code-split/indirectly-callable.ll
Outdated
Show resolved
Hide resolved
llvm/test/tools/sycl-post-link/virtual-functions/module-cleanup-comdat.ll
Outdated
Show resolved
Hide resolved
llvm/test/tools/sycl-post-link/virtual-functions/module-cleanup.ll
Outdated
Show resolved
Hide resolved
…ual-functions-device-img-properties
There was a problem hiding this 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
…ual-functions-device-img-properties
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. |
Implementation design explaining those changes in a bigger picture can be found in #10540
Key things implemented here:
calls-indirectly
LLVM IR attribute for kernels that construct objects with virtual functions, but don't do callsEven 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.