Skip to content

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.

//
//===----------------------------------------------------------------------===//

// 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!

Jakub Chlanda 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 jchlanda temporarily deployed to WindowsCILock July 2, 2025 05:33 — with GitHub Actions Inactive
@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
uditagarwal97 pushed a commit that referenced this pull request Jul 16, 2025
This PR brings back #18918 and #19302, and fixes the issue with shared
library builds.

The problem was that we accessed hidden symbols defined in headers from
the `clang/lib` directory to obtain paths to the vendor-specific device
library files. We now use the `ToolChain::getDeviceLibs` API, and supply
a minimal implementation for the `CudaToolchain`.

---------

Signed-off-by: Julian Oppermann <[email protected]>
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