Skip to content

[SYCL] Fail on kernel lambda size mismatch #6374

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

Merged
merged 3 commits into from
Jul 8, 2022

Conversation

steffenlarsen
Copy link
Contributor

Some host-compilers may create lambdas with captures that do not match the ones used when extracting kernel descriptors. The compiler does not currently have a stable way of handling these cases, so instead this PR adds a static assertion that the sizes match with a message informing about the limitation.

Some host-compilers may create lambdas with captures that do not match
the ones used when extracting kernel descriptors. The compiler does not
currently have a stable way of handling these cases, so instead this PR
adds a static assertion that the sizes match with a message informing
about the limitation.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen steffenlarsen marked this pull request as ready for review June 29, 2022 14:20
@steffenlarsen steffenlarsen requested a review from a team as a code owner June 29, 2022 14:20
@steffenlarsen steffenlarsen requested a review from v-klochkov June 29, 2022 14:20
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
using type = FalseT;
};

using int64_t = conditional<sizeof(long) == 8, long, long long>::type;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally we should check that sizeof(long long) is 8, if sizeof(long) != 8.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but this mimics the decision made by SemaSYCL when generating the specialization, so it should be fine for our testing.

@steffenlarsen
Copy link
Contributor Author

@v-klochkov - Friendly ping.

@@ -13,6 +13,8 @@
#include <CL/sycl/detail/defines_elementary.hpp>
#include <CL/sycl/detail/export.hpp>

#include <cstdint>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, perhaps this will also fix one of internal trackers reporting problems with undefined int8_t type slipped into integration-header file.

Copy link
Contributor

Choose a reason for hiding this comment

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

This inclusion is giving oneapi/dpl a stomach ache.

Copy link
Contributor

Choose a reason for hiding this comment

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

@v-klochkov v-klochkov merged commit d278c67 into intel:sycl Jul 8, 2022
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.

5 participants