Skip to content

Conversation

@asavonic
Copy link
Contributor

Signed-off-by: Andrew Savonichev [email protected]

@asavonic asavonic requested a review from AlexeySachkov July 31, 2019 10:45
// attribute must never appear as a group" for attributes like cold and hot.
if (!D->hasAttr<OpenCLKernelAttr>()) {
if (!(D->hasAttr<OpenCLKernelAttr>() ||
LangOpts.SYCLIsDevice || LangOpts.SYCLIsHost)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

LangOpts.SYCLIsHost variables name suggests that it's true for the host side of the SYCL application.
Could you clarify what should happen with this attribute on the host side? How will handle it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LangOpts.SYCLIsHost variables name suggests that it's true for the host side of the SYCL application.

Correct.

Could you clarify what should happen with this attribute on the host side? How will handle it?

The reason I did this was to avoid a diagnostic about unsupported attributes for host compilation. Since I didn't do anything to drop the attribute, it is probably emitted to LLVM IR (and ignored by optimizations & codegen) for host code.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually re-using of solution developed in #296

Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK this attribute cannot be ignored, so we need somehow handle it when executing on host device.

@bader bader merged commit 68578d7 into intel:sycl Aug 2, 2019
vladimirlaz pushed a commit to vladimirlaz/llvm that referenced this pull request Jan 31, 2020
  CONFLICT (content): Merge conflict in clang/include/clang/Driver/Options.td
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.

4 participants