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

[SYCL2020] Add experimental support for optional lambda kernel naming #281

Merged
merged 1 commit into from
Aug 6, 2020

Conversation

illuhad
Copy link
Collaborator

@illuhad illuhad commented Jul 27, 2020

This (re-)introduces support for optional lambda kernel naming when using clang >= 10.

About kernel naming in hipSYCL

Because the HIP/CUDA programming models do not require kernel naming (even when instantiating a __global__ kernel with a lambda), it may be surprising that hipSYCL requires lambda kernel naming at all, and indeed, early versions of hipSYCL did not require it. Then came issue #49, where we noticed that clang's HIP/CUDA frontend would not enumerate kernel lambdas consistently across host and device passes, which can potentially cause mismatches in the mangled kernel names between host and device. This in turn can create spurious, and silent kernel launch failures that are very hard to reproduce and very frustrating to debug.
To circumvent this issue, mandatory kernel naming as described in the SYCL specification was introduced to hipSYCL.

How this PR works

Originally, my intention was to borrow code from DPC++, which has support for generating unique names for lambda functions. However, due to the way the clang name mangling code is structured, we would have needed to pull in several thousand lines of code from Intel's clang fork, which is impossible to maintain and get working across the multiple clang versions we support.

Thankfully, it seems that in the meantime the good folk working on clang CUDA/HIP have addressed the original issue from clang 10 onwards, so this turned out to be easier than expected:
https://reviews.llvm.org/D68818

This means that if clang is sufficiently new, we can just ignore if the user did not provide a kernel name and hope that clang mangles the kernel correctly.

Because the original issue #49 can be very spurious and hard to reproduce, we should still consider this support somewhat experimental as I might simply have been lucky in my testing.

If it turns out that the issue is still present in upstream clang, we will have to resort to plan B. Because of how name mangling works in clang we cannot just switch out the lambda part that we are interested in - So we would need to partially demangle the clang-generated kernel name, identify all lambda functions and replace their enumerations with a conglomerate of the the line/column numbers of the definition of all involved lambda functions. Sounds annoying, and probably is, so let's cross our fingers that this here works :)

@illuhad illuhad merged commit d111196 into develop Aug 6, 2020
@illuhad illuhad deleted the feature/sycl-2020/optional-kernel-names branch August 6, 2020 11:58
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.

1 participant