Skip to content

[SYCL] Fix diagnostic about non-globally-visible kernel name #928

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

Conversation

AlexeySachkov
Copy link
Contributor

Moved some dead code from CodeGenSYCL/int_header1.cpp to the new test

@bader
Copy link
Contributor

bader commented Feb 8, 2020

Ping.

@AlexeySachkov AlexeySachkov force-pushed the private/asachkov/fix-error-about-unnamed-lambda branch from 642d069 to b3a10b3 Compare February 18, 2020 12:27
@AlexeySachkov AlexeySachkov force-pushed the private/asachkov/fix-error-about-unnamed-lambda branch from b3a10b3 to 2180f4e Compare February 18, 2020 14:36
kernel_single_task<KernelName2>([=]() { acc.use(); });
#endif

#ifdef LD__
Copy link
Contributor

@kbobrovs kbobrovs Feb 19, 2020

Choose a reason for hiding this comment

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

I suggest to convert these ("expected compilation error") to negative compilation tests together with removal. Or leave it as is if there are no resource now.

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 preserved these negative test cases in the new test, this is mentioned in commit message

});

#ifndef __SYCL_UNNAMED_LAMBDA__
// expected-error@16 {{kernel needs to have a globally-visible name}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to make this diagnostic a bit more informative. It should point to kernel-name class definitions and defective kernel invoke call both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@bader
Copy link
Contributor

bader commented Feb 28, 2020

@AlexeySachkov, please, address comments.

@AlexeySachkov AlexeySachkov force-pushed the private/asachkov/fix-error-about-unnamed-lambda branch from 2180f4e to 5b8496b Compare March 20, 2020 16:24
bader
bader previously approved these changes Mar 20, 2020
Fznamznon
Fznamznon previously approved these changes Mar 20, 2020
Copy link
Contributor

@Fznamznon Fznamznon 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 Mar 21, 2020

@AlexeySachkov, please, fix LIT tests.

Moved some dead code from CodeGenSYCL/int_header1.cpp to the new test

Signed-off-by: Alexey Sachkov <[email protected]>
Signed-off-by: Alexey Sachkov <[email protected]>
@AlexeySachkov AlexeySachkov dismissed stale reviews from Fznamznon and bader via d410017 March 23, 2020 09:21
@AlexeySachkov AlexeySachkov force-pushed the private/asachkov/fix-error-about-unnamed-lambda branch from edddd29 to d410017 Compare March 23, 2020 09:21
@AlexeySachkov
Copy link
Contributor Author

@AlexeySachkov, please, fix LIT tests.

Should be done now

@bader bader requested a review from Fznamznon March 23, 2020 09:31
@bader bader merged commit fadaa59 into intel:sycl Mar 24, 2020
@AlexeySachkov AlexeySachkov deleted the private/asachkov/fix-error-about-unnamed-lambda branch April 1, 2020 10:23
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