Skip to content

[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

Merged
merged 38 commits into from
Jul 2, 2025
Merged

Conversation

jchlanda
Copy link
Contributor

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.

Copy link
Contributor

@jopperm jopperm left a 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)
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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

Copy link
Contributor

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!

jchlanda added 3 commits June 12, 2025 17:30
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
@jchlanda
Copy link
Contributor Author

jchlanda commented Jul 2, 2025

@jopperm the job's finally green and ready for review.

Copy link
Contributor

@jopperm jopperm 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!

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.

@jchlanda
Copy link
Contributor Author

jchlanda commented Jul 2, 2025

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.

@jchlanda
Copy link
Contributor Author

jchlanda commented Jul 2, 2025

@intel/llvm-gatekeepers this should be ready to roll. Thank you.

@sommerlukas sommerlukas merged commit 6d97d98 into intel:sycl Jul 2, 2025
38 of 39 checks passed
mikolaj-pirog added a commit that referenced this pull request Jul 3, 2025
mikolaj-pirog added a commit that referenced this pull request Jul 4, 2025
steffenlarsen pushed a commit that referenced this pull request Jul 7, 2025
…19304)

This reverts commit 6d97d98.

The commit causes failure in shared-libs build:
#19291

---------

Co-authored-by: Nicolas Miller <[email protected]>
mikolaj-pirog added a commit that referenced this pull request Jul 7, 2025
…19304)

This reverts commit 6d97d98.

The commit causes failure in shared-libs build:
#19291

---------

Co-authored-by: Nicolas Miller <[email protected]>
jopperm added a commit to jopperm/llvm that referenced this pull request Jul 8, 2025
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.

4 participants