Skip to content

[SYCL][NFC] Throw warning when sycl.hpp is included without -fsycl flag. #19279

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 4 commits into from
Jul 4, 2025
Merged

[SYCL][NFC] Throw warning when sycl.hpp is included without -fsycl flag. #19279

merged 4 commits into from
Jul 4, 2025
+36 −1

Conversation

uditagarwal97
Copy link
Contributor

To improve user experience.

Fixes #12052

@uditagarwal97 uditagarwal97 self-assigned this Jul 2, 2025
@uditagarwal97 uditagarwal97 requested a review from a team as a code owner July 2, 2025 17:32
steffenlarsen
steffenlarsen reviewed Jul 2, 2025
View reviewed changes
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

I'm a little split on this. With all the new ways of doing RTC and offline compiling SYCL code, could we think of cases where users might actually want to include sycl.hpp without -fsycl? Can we maybe offer them a preprocessor macro to define to switch off this warning?

@uditagarwal97 uditagarwal97 force-pushed the private/udit/fsyclmacro branch from 9d01384 to efbac5b Compare July 3, 2025 16:07
uditagarwal97
uditagarwal97 commented Jul 3, 2025
View reviewed changes
sycl/include/sycl/sycl.hpp
#define STRINGIFY(x) #x
#define TOSTRING(x) STRINGIFY(x)

#ifdef _MSC_VER
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to reviewers: Here's a working example of warnings being thrown for different compilers: https://godbolt.org/z/bvoP3q3Gx

@uditagarwal97
Copy link
Contributor Author

uditagarwal97 commented Jul 3, 2025 •
edited
Loading

I'm a little split on this. With all the new ways of doing RTC and offline compiling SYCL code, could we think of cases where users might actually want to include sycl.hpp without -fsycl? Can we maybe offer them a preprocessor macro to define to switch off this warning?

Totally. User could be intentionally including sycl.hpp without -fsycl flag, like for compilation of host-only APIs (like what we do in sycl-ls tool). I've added a macro SYCL_DISABLE_FSYCL_SYCLHPP_WARNING to disable the warning.

steffenlarsen
steffenlarsen reviewed Jul 3, 2025
View reviewed changes
sycl/include/sycl/sycl.hpp
#endif

#if !defined(SYCL_LANGUAGE_VERSION) && \
!defined(SYCL_DISABLE_FSYCL_SYCLHPP_WARNING)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this! Should we mention it in sycl/doc/PreprocessorMacros.md?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in 8efc70e

@uditagarwal97 uditagarwal97 requested a review from a team as a code owner July 3, 2025 16:26
steffenlarsen
steffenlarsen approved these changes Jul 4, 2025
View reviewed changes
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

LGTM!

@uditagarwal97
Copy link
Contributor Author 

********************
Failed Tests (1):
  SYCL :: Basic/handler/handler_mem_op.cpp


Testing Time: 195.61s

Total Discovered Tests: 2442
  Excluded         :  584 (23.91%)
  Unsupported      :  431 (17.65%)
  Passed           : 1424 (58.31%)
  Expectedly Failed:    2 (0.08%)
  Failed           :    1 (0.04%)

Created #19305 to track flaky Arc test failure. This PR just adds a compiler warning and should not be anyhow related to the failing test.

@uditagarwal97 uditagarwal97 merged commit 79d1e54 into sycl Jul 4, 2025
25 of 26 checks passed
@uditagarwal97 uditagarwal97 deleted the private/udit/fsyclmacro branch July 4, 2025 13:35
@uditagarwal97 uditagarwal97 mentioned this pull request Jul 4, 2025
Merged
uditagarwal97 added a commit that referenced this pull request Jul 4, 2025
@uditagarwal97
[SYCL][NFC] Disable sycl/sycl.hpp warning in unittests (#19310)
6b67abd 
After #19279, `ninja check-sycl`
produces hundreds of warning about including `sycl.hpp` without `-fsycl`
flag."
This PR disables those warnings for unittests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexeySachkov AlexeySachkov AlexeySachkov approved these changes

@steffenlarsen steffenlarsen steffenlarsen approved these changes

Assignees

@uditagarwal97 uditagarwal97

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Better diagnostic: suggest using -fsycl flag when sycl/sycl.hpp is included
3 participants
@uditagarwal97 @AlexeySachkov @steffenlarsen