Skip to content
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] Fix devicelib identification for NVPTX #15357

Merged
merged 6 commits into from
Sep 12, 2024

Conversation

frasercrmck
Copy link
Contributor

@frasercrmck frasercrmck commented Sep 11, 2024

Recent upstream changes removed the workaround that required libraries to be named ".cubin".

We also renamed the NVPTX libdevice library when we unified it into a single bytecode library.

Unfortunately both of these things broke the driver's brittle logic that determined whether or not an object was a device library. This resulted in us not stripping any symbols from the final object, which is both slower and broke some SYCL CTS tests.

This commit more clearly delineates logic between NVPTX and NativeCPU, as the latter target didn't actually link against anything containing 'libdevice' so was too lenient.

Recent upstream changes removed the workaround that required libraries
to be named ".cubin".

We also renamed the NVPTX libdevice library when we unified it into a
single bytecode library.

Unfortunately both of these things broke the driver's brittle logic that
determined whether or not an object was a device library. This resulted
in us not stripping any symbols from the final object, which is both
slower and broke some SYCL CTS tests.

This commit more clearly delineates logic between NVPTX and Native-CPU,
as the latter target didn't actually link against anything containing
'libdevice' so was too lenient.
clang/test/Driver/sycl-nvptx-link.cpp Outdated Show resolved Hide resolved
clang/test/Driver/sycl-nvptx-link.cpp Outdated Show resolved Hide resolved
@frasercrmck
Copy link
Contributor Author

Unexpected passing test

Unexpectedly Passed Tests (1):
  SYCL :: OneapiDeviceSelector/no_duplicate_devices.cpp

see #15341 and #15288

Copy link
Contributor

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

@frasercrmck
Copy link
Contributor Author

@intel/llvm-gatekeepers this is ready to merge, thanks - there's one unrelated unexpected pass

@martygrant martygrant merged commit 8d329a0 into intel:sycl Sep 12, 2024
12 of 13 checks passed
@frasercrmck frasercrmck deleted the fix-nvptx-link branch September 12, 2024 15:57
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.

3 participants