Skip to content

Conversation

sergey-semenov
Copy link
Contributor

Static library linking mechanism uses symbols pulled in by the host linker to determine what parts of device code to include during its linking. The exceptions thrown from nd_item on host interfere with that, since all the following host code for the kernel can get optimized away.

This change removes those exceptions since nd_item can't be constructed on the host side.

Static library linking mechanism uses symbols pulled in by the host
linker to determine what parts of device code to include during its
linking. The exceptions thrown from nd_item on host interfere with
that, since all the following host code for the kernel can get optimized
away.

This change removes those exceptions since nd_item can't be constructed
on the host side.
@sergey-semenov
Copy link
Contributor Author

@intel/dpcpp-tools-reviewers Could you please have a look?

Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, quick q on the test but not a blocker

Copy link
Contributor

@asudarsa asudarsa 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

@bader
Copy link
Contributor

bader commented Jun 25, 2024

@sergey-semenov, FYI. 7999e27 also breaks debugger tests when SYCL device is CPU. Probably the best solution at the moment is to revert 7999e27 to fix both problems at once (not sure if it's possible due to ABI break though).

@sergey-semenov
Copy link
Contributor Author

@sergey-semenov, FYI. 7999e27 also breaks debugger tests when SYCL device is CPU. Probably the best solution at the moment is to revert 7999e27 to fix both problems at once (not sure if it's possible due to ABI break though).

Thanks for letting me know. I'd still rather merge this first even if we do end up reverting the stateless change since the ABI breaking window is two days away (I think?) and this is having a big impact on some of our users.

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.

6 participants