-
Notifications
You must be signed in to change notification settings - Fork 787
[SYCL] Disable std::byte code by _HAS_STD_BYTE #4834
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
Conversation
MS VS allows to disable std::byte type using __HAS_STD_BYTE=0 (https://devblogs.microsoft.com/cppblog/c17-features-and-stl-fixes-in-vs-2017-15-3/). This patch disables SYCL code which relies on this type in this case.
// RUN: %clangxx -fsycl -fsyntax-only -Xclang -verify -D_HAS_STD_BYTE=0 %s -Xclang -verify-ignore-unexpected=note,warning | ||
// RUN: %clangxx -fsycl -fsyntax-only -Xclang -verify %s -Xclang -verify-ignore-unexpected=note,warning | ||
// expected-no-diagnostics | ||
#include <CL/sycl.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I missed that the first time. I guess the problem is exposed on Windows platform only. Am I right?
#include <CL/sycl.hpp> | |
// REQUIRES: windows | |
#include <CL/sycl.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. But running both on Linux and Windows ensures that Linux is not regressed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not blocking, but this test is useless on Linux. It does nothing useful.
You might invent infinite number of other defines to tests if they are not regressed as well. What might be useful to dump all defines enabled by including sycl.hpp
and check there is nothing unexpected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Macro definition name in the description uses two underscores instead of one. Please, fix before merging.
@againull Could you please review? |
MS VS allows to disable std::byte type using _HAS_STD_BYTE=0
(https://devblogs.microsoft.com/cppblog/c17-features-and-stl-fixes-in-vs-2017-15-3/).
This patch disables SYCL code which relies on this type in this case.