-
Notifications
You must be signed in to change notification settings - Fork 793
[SYCL] RTC support for GPU targets #18918
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
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.
Approach LGTM, just a couple of nits.
@@ -6,7 +6,6 @@ | |||
// | |||
//===----------------------------------------------------------------------===// | |||
|
|||
// REQUIRES: (opencl || level_zero) |
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.
Can/should we enable the other E2E tests in this directory as well for third-party GPUs?
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.
We should, that has escaped my attention. Let me enable the tests locally and see what happens. If they work out of the box or require little effort to fix I'll update this PR, if not I'll do it in a follow up.
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.
It would be awesome, amusing, and possibly even confusing if the "AoT-Only" targets work with KernelCompiler.
<3 <3 <3
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.
If they work out of the box or require little effort to fix I'll update this PR, if not I'll do it in a follow up.
SGTM!
This patch extends RTC support to GPU (AMD and Nvidia) targets. Additionally: * reinstate __SYCL_PROGRAM_METADATA_TAG_NEED_FINALIZATION tag, * split sycl.cpp RTC file to exclude IMF from the body of the main test.
* sycl_imf fix - commented kernel * move away from ifdefs * don't use CLI strings for args in setting up the GPU targets
@jopperm the job's finally green and ready for review. |
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!
Even though the persistent bitcode cache is target-independent (and the target-specific flags are baked correctly into the hash value), we should consider splitting the eviction testing off of the sycl_cache.cpp
E2E test in a follow-up PR, to be able to run the latter on third-party GPUs as well.
Sounds good. I'll do it in a new PR. Thank you. |
@intel/llvm-gatekeepers this should be ready to roll. Thank you. |
This reverts commit 6d97d98.
This reverts commit 6d97d98.
…19304) This reverts commit 6d97d98. The commit causes failure in shared-libs build: #19291 --------- Co-authored-by: Nicolas Miller <[email protected]>
…19304) This reverts commit 6d97d98. The commit causes failure in shared-libs build: #19291 --------- Co-authored-by: Nicolas Miller <[email protected]>
…ntel#18918)" (intel#19304)" This reverts commit 29e7b63.
This patch extends RTC support to GPU (AMD and Nvidia) targets.
Additionally: