Skip to content

[SYCL] Add host kernel instantiation for debuggers #15256

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

steffenlarsen
Copy link
Contributor

The changes in #14460 removed the seemingly unused functions for running kernels on on host. However, this turned out to be used by debuggers as they need the kernel code to be in the host executable.

This commit adds a simplified version of the kernel instantiation that the aforementioned patch removed.

The changes in intel#14460 removed the
seemingly unused functions for running kernels on on host. However, this
turned out to be used by debuggers as they need the kernel code to be
in the host executable.

This commit adds a simplified version of the kernel instantiation that
the aforementioned patch removed.

Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen
Copy link
Contributor Author

Timeout in host_task_unsampled_image_write is unrelated.

@steffenlarsen
Copy link
Contributor Author

Ping @bso-intel | @intel/llvm-reviewers-runtime

@@ -154,6 +154,8 @@ runKernelWithArg(KernelType KernelName, ArgType Arg) {
// The pure virtual class aimed to store lambda/functors of any type.
class HostKernelBase {
public:
// NOTE: InstatitateKernelOnHost() should not be called.
virtual void InstatitateKernelOnHost() = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be

Suggested change
virtual void InstatitateKernelOnHost() = 0;
virtual void InstantiateKernelOnHost() = 0;

?
And also for a comment above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in #15300

Copy link
Contributor

@dm-vodopyanov dm-vodopyanov left a comment

Choose a reason for hiding this comment

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

Agreed to fix a typo separately

@steffenlarsen steffenlarsen merged commit 2e777b5 into intel:sycl Sep 5, 2024
13 of 14 checks passed
steffenlarsen added a commit to steffenlarsen/llvm that referenced this pull request Sep 6, 2024
The changes in intel#15256 unintentionally
broke ABI by reordering the virtual member functions of HostKernelBase.
This commit amends this by moving the new function to the end of the
new member function.

Additionally, this fixes a typo in the new function.

Signed-off-by: Larsen, Steffen <[email protected]>
againull pushed a commit that referenced this pull request Sep 6, 2024
The changes in #15256 unintentionally
broke ABI by reordering the virtual member functions of HostKernelBase.
This commit amends this by moving the new function to the end of the new
member function.

Additionally, this fixes a typo in the new function.

---------

Signed-off-by: Larsen, Steffen <[email protected]>
Co-authored-by: Marcos Maronas <[email protected]>
AlexeySachkov pushed a commit to AlexeySachkov/llvm that referenced this pull request Nov 26, 2024
The changes in intel#14460 removed the
seemingly unused functions for running kernels on on host. However, this
turned out to be used by debuggers as they need the kernel code to be in
the host executable.

This commit adds a simplified version of the kernel instantiation that
the aforementioned patch removed.

Signed-off-by: Larsen, Steffen <[email protected]>
AlexeySachkov pushed a commit to AlexeySachkov/llvm that referenced this pull request Nov 26, 2024
)

The changes in intel#15256 unintentionally
broke ABI by reordering the virtual member functions of HostKernelBase.
This commit amends this by moving the new function to the end of the new
member function.

Additionally, this fixes a typo in the new function.

---------

Signed-off-by: Larsen, Steffen <[email protected]>
Co-authored-by: Marcos Maronas <[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.

2 participants