Skip to content

[SYCL] Remove diagnostics emission from Integration header functionality #2474

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Sep 18, 2020
Merged

[SYCL] Remove diagnostics emission from Integration header functionality #2474

merged 13 commits into from
Sep 18, 2020

Conversation

srividya-sundaram
Copy link
Contributor

@srividya-sundaram srividya-sundaram commented Sep 15, 2020 •
edited
Loading

Using Clang's Visitor classes to walk the template type parameter - KernelName to emit the following diagnostics:

  • Kernel name is invalid. Unscoped enum requires fixed underlying type - enum in template params
  • Kernel needs to have a globally-visible name
  • Kernel name is missing

@srividya-sundaram srividya-sundaram changed the title [SYCL] Remove dignostics emission from Integration header functionality [SYCL][WIP] Remove dignostics emission from Integration header functionality Sep 15, 2020
@Fznamznon
Copy link
Contributor

@srividya-sundaram , please update your branch, it seems quite old, so even buildbots cannot checkin your changes.

Fznamznon
Fznamznon reviewed Sep 15, 2020
View reviewed changes
@srividya-sundaram srividya-sundaram removed the request for review from erichkeane September 16, 2020 04:25
Fznamznon
Fznamznon reviewed Sep 16, 2020
View reviewed changes
Fznamznon
Fznamznon reviewed Sep 17, 2020
View reviewed changes
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.

I think we need to convert this PR from draft to regular PR and proceed with review together with other codeowners.

@srividya-sundaram srividya-sundaram changed the title [SYCL][WIP] Remove dignostics emission from Integration header functionality [SYCL] Remove dignostics emission from Integration header functionality Sep 17, 2020
@Fznamznon Fznamznon changed the title [SYCL] Remove dignostics emission from Integration header functionality [SYCL] Remove diagnostics emission from Integration header functionality Sep 17, 2020
@srividya-sundaram srividya-sundaram marked this pull request as ready for review September 17, 2020 14:57
@Fznamznon Fznamznon linked an issue Sep 17, 2020 that may be closed by this pull request
[SYCL] Diagnostics should be generated at point of kernel invocation #1706
Closed
Fznamznon
Fznamznon previously approved these changes Sep 17, 2020
View reviewed changes
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.

Looks good to me. But I'd like @elizabethandrews and @premanandrao to take a look as well.

premanandrao
premanandrao reviewed Sep 17, 2020
View reviewed changes
elizabethandrews
elizabethandrews approved these changes Sep 18, 2020
View reviewed changes
Copy link
Contributor

@elizabethandrews elizabethandrews left a comment

Choose a reason for hiding this comment

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

LGTM except for small nit

@romanovvlad romanovvlad merged commit a09aed0 into intel:sycl Sep 18, 2020
@srividya-sundaram srividya-sundaram mentioned this pull request Oct 6, 2020
Merged
kbenzie pushed a commit to kbenzie/intel-llvm that referenced this pull request Feb 17, 2025
@martygrant
Merge pull request intel#2474 from nrspruit/fix_use_after_free_build_…
9c816fe 
…strings

[L0] Fix use after free with Module build strings
Chenyang-L pushed a commit that referenced this pull request Feb 18, 2025
@martygrant
Merge pull request #2474 from nrspruit/fix_use_after_free_build_strings
f09091a 
[L0] Fix use after free with Module build strings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elizabethandrews elizabethandrews elizabethandrews approved these changes

@erichkeane erichkeane Awaiting requested review from erichkeane

@premanandrao premanandrao Awaiting requested review from premanandrao

@Fznamznon Fznamznon Awaiting requested review from Fznamznon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[SYCL] Diagnostics should be generated at point of kernel invocation
6 participants
@srividya-sundaram @Fznamznon @erichkeane @premanandrao @elizabethandrews @romanovvlad