Skip to content

[SYCL] Conditionally enable diagnosing that requires integration header #5044

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 4 commits into from
Dec 2, 2021

Conversation

Fznamznon
Copy link
Contributor

#4945 introduced a new diagnostic that
works correcly only if integration header is included. However it is
possible that -fsycl-is-host flag can be used without integration
header, so the diagnostic should be issued only if integration header is
incuded. This patch adds new cc1 option that helps to understand that.

intel#4945 introduced a new diagnostic that
works correcly only if integration header is included. However it is
possible that `-fsycl-is-host` flag can be used without integration
header, so the diagnostic should be issued only if integration header is
incuded. This patch adds new cc1 option that helps to understand that.
mdtoguchi
mdtoguchi previously approved these changes Nov 29, 2021
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.

Actually, thinking a bit more - EnableIntHeader might be confusing here. I think we should rename this to EnableIntHeaderDiag or something

@Fznamznon Fznamznon changed the title [SYCL] Conditionally enable diagnosing that require integration header [SYCL] Conditionally enable diagnosing that requires integration header Nov 30, 2021
@Fznamznon Fznamznon dismissed stale reviews from mdtoguchi and elizabethandrews via 20920a3 November 30, 2021 11:52
Copy link
Contributor

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

2 nits, otherwise LGTM

@Fznamznon Fznamznon marked this pull request as ready for review November 30, 2021 15:43
Copy link
Contributor

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM

@Fznamznon Fznamznon requested a review from mdtoguchi November 30, 2021 15:51
@Fznamznon
Copy link
Contributor Author

Can this one be merged? It appears to be quite important for the downstream.

@bader
Copy link
Contributor

bader commented Dec 2, 2021

/summary:run

@Fznamznon
Copy link
Contributor Author

Fznamznon commented Dec 2, 2021

/summary:run

The tests have passed.

@bader bader merged commit 2fa6376 into intel:sycl Dec 2, 2021
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.

9 participants